feat(review): back pr-context on Aone Code targets - #9621
Conversation
pr-context was the one read subcommand still gh-direct, so every Aone run was forced context-unavailable: the verdict capped at COMMENT (the wired a1 approval could never fire), Agent 0 skipped, and the machine ledger never recovered from posted summaries. Route it through the platform reader with a normalized context bundle; Aone serves it from mr view + the flat comment list (thread comments carry the ledger), GitHub's implementation is an extraction of the existing calls — its output stays byte-identical. The forced cap leaves the Aone write path for parity with GitHub's state-claim handling, and the refetch commands a context file emits bake --pr on Aone, where comment bodies are addressed per-MR.
E2E test reportPlan: Baseline (global release 0.21.14)
Post-change (local bundle)
Environment note: the authoring machine's a1/ncs credential chain degraded intermittently during the run ( New defect found (pre-existing, out of scope): branch-based MRs carry a branch name in |
|
Thanks for the PR! Re-run — gate re-checked against the current head Template looks good ✓ Problem: observed gap, not theoretical — unchanged since the last pass: Direction: aligned, unchanged — next step of the established series (platform abstraction #9096, Aone read path #9226, Aone write path #9491), follow-ups tracked separately (#9613–#9620). CHANGELOG: no direct reference yet, but the area is clearly in flight and relevant. Size: core paths touched — breakdown at the current head: 703 production logic lines (aone.ts 219, pr-context.ts 261, github.ts 104, types.ts 75, submit.ts 29, bundled review SKILL.md prose 15), 858 test lines, 299 docs lines, 2 lines of CI config (the size-baseline correction). The Approach: still matches the stated goal. The two new commits are both review-driven: one closes the last two recorded round-7 deferrals (the comment-body refetch now reads the same resolved-inclusive union the context render reads, and the refetch path gets the envelope shape guard), the other corrects a wrong CI size-baseline value that made the ratchet trip on a file this branch doesn't grow. Both main merges integrated cleanly — the PR's diff against the refreshed base is byte-accounting-identical to the last pass except exactly this delta, so no conflict resolution silently touched the PR's logic. Risk: no elevated risk signals — no revert-correlated paths touched. Moving on to code review. 🔍 中文说明感谢贡献!Re-run——已按当前 head 模板完整 ✓ 问题:已观测到的缺口,不是理论问题——与上次审查一致:shipping release 上 方向:对齐,不变——既定系列的下一步(平台抽象 #9096、Aone 读路径 #9226、Aone 写路径 #9491),后续工作单独跟踪(#9613–#9620)。 规模:触及核心路径——当前 head 拆分:703 行生产逻辑(aone.ts 219、pr-context.ts 261、github.ts 104、types.ts 75、submit.ts 29、bundled review SKILL.md 文本 15)、858 行测试、299 行文档、2 行 CI 配置(size-baseline 修正)。 方案:仍与目标匹配。两个新提交均为评审驱动:一个关闭上一轮记录在案的两条第 7 延后项(comment-body 补取改读与 context 渲染相同的含 resolved 并集,补取路径获得信封形状守卫),另一个修正了一个错误的 CI size-baseline 数值——该错误使 ratchet 在一个本分支未增长的文件上误触发。两次 main 合并均干净集成——相对刷新后 base 的 PR diff 与上次审查逐行数一致,除恰好这一 delta 外别无变化,故无冲突解决悄然触碰 PR 逻辑。 风险:无升级风险信号——未触及 revert 相关路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run — this pass reviews the two commits landed since the last pass ( My independent proposal for the delta — the comment-body refetch must read the SAME comment surface the context render reads, so extract the shape-checked default+resolved union into one helper and point both sites at it — is exactly what What the new commits closed, verified against the recorded round-7 deferrals:
The round-9 sequenceDiagram
participant P1 as review pr-context
participant P2 as platform registry
participant P3 as selected reader (gh or a1)
participant P4 as renderer and ledger recovery
P1->>P2: detect platform (--host flag, else clone origin)
P2-->>P1: reader
P1->>P3: ensureAuthenticated, getReviewContext, getCurrentUser
P3-->>P1: normalized bundle (metadata, comments, verdicts, carriers)
Note over P3: Aone unions default and --resolved listings, deduped by id
Note over P3: the comment-body refetch reads the same union
P1->>P4: split channels by path, recover ledger, fail-closed identity
P4-->>P1: context file, ledger side file, refetch commands
Files changed (14 of 14)
TestingUnattended CI run — PR code is never built or executed here; the evidence below is the PR's own CI for the reviewed commit, fetched via the API. All checks on
One row per check name (latest run); skipped checks omitted; the merge-queue-only legs are named in the prose above. / 每个检查名一行(取最新一次运行),省略 skipped;仅合并队列运行的腿在正文中说明。 Not verified here: the live Aone leg (a1 CLI against gitlab.alibaba-inc.com) — this environment has no a1 credentials, so the E2E results in the PR body (exit 0 on MR 29408570, refetch round-trip, byte-identical GitHub output on PR 9491) remain the author's claim, not independently re-run; the new resolved-refetch behaviour is likewise pinned only by unit witnesses here. The byte-identical-GitHub claim rests on the extraction leaving the transport seam unchanged plus the unmodified suite passing — decent, but unit-level, evidence. Sandboxed verification would settle part of this: 中文说明代码审查:本次为 re-run——复审上次审查后落地的两个提交( 测试:无人值守 CI 运行——此处从不构建或执行 PR 代码;证据来自 API 拉取的该 commit 自身 CI。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the delta since the last pass is small, review-driven, and closes exactly the two deferrals the previous pass recorded, with witnesses that fail the old behaviour; what keeps this from 5 is unchanged: the live Aone leg rests on the author's report (no a1 credentials exist in this environment to re-run it). Stepping back: the picture is the same as the last pass, only more settled. The problem was reproduced against the shipping release; the direction is the next step of a series whose read and write paths already landed; the implementation remains the minimum the goal needs. The new fix commit is the kind of change I want to see at this stage of a PR — not new surface, but the refetch path brought onto the same union the render path reads, so the two sides cannot drift by construction, and the envelope guard lifted into the shared helper instead of duplicated. My independent proposal for the delta matches what landed; I didn't find a simpler path it missed. I scrutinized three things specifically this pass: the eager-union consequence (a failing For the maintainer's awareness: the standing Reservations, named plainly: the live end-to-end leg (real MR, refetch round-trip — now including a resolved-comment refetch — byte-identical GitHub output) is author-reported, and the Verdict: approve. All CI on the reviewed commit is green and no PR-CI run is pending, so the approval posts now, pinned to the reviewed commit. 中文说明置信度:4/5 —— 上次审查之后的 delta 很小、完全由评审驱动,恰好关闭了上一轮记录在案的两条延后项,且配有钉住旧行为的见证测试;让它不到 5 分的理由不变:Aone 实测腿仍只有作者报告(本环境无 a1 凭证,无法复跑)。 退一步看:整体图景与上次一致,且更趋定型。问题已在 shipping release 上复现;方向是读/写路径均已落地的系列的下一步;实现仍是目标所需的最小改动。新修复提交正是 PR 走到这个阶段我想看到的改动——不是新增面,而是把补取路径挪到与渲染路径相同的并集上,使两侧构造上不可能漂移;信封守卫被提升进共享 helper 而非复制。我对这一 delta 的独立解法与落地一致,没有找到更简路径。 本次我重点核了三处:急切并集的后果( 提请维护者注意:本 PR 上悬置的 保留意见,直说:实测腿(真实 MR、补取往返——现在还包括 resolved 评论补取——、GitHub 输出逐字节一致)为作者报告,Stage 2 点名的 结论:approve。该 commit 的 CI 全绿且无 PR CI 在途,批准现在发布并固定到被审查的 commit。 — Qwen Code · qwen3.8-max Reviewed at |
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.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
@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 冲突,直到移除标签或达到轮次上限。移除 |
|
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 |
…9621) On Aone this pipeline's own round summaries are path-less comments, so they ride pr-context's issue channel, where their visible **[Critical]** lines self-promoted every prior Critical-bearing summary into "Blockers to re-check" — rendering each prior Critical three times (beside the ledger section and the inline roots that own the same findings) and spending the section budget on the pipeline's own prose until genuine human blockers degraded to snippets. Exclude bodies carrying the ledger marker from issue-channel promotion and the stdout count, strip the marker out of the settled snippet, and switch the pr_number guard to the canonical isPositivePrNumber so 0x10/5. spellings cannot fragment side-file continuity. Pin the witnesses the round's findings name: the guard, args.host forwarding, the issue-kind --pr refetch branch, the account-first author keying, and the GitHub test suites' independence from the cwd-origin probe.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9621Commit: Dispositions
Mutation probes (each new guard/branch witnessed)
VerificationCommands actually run and their results (final committed state):
中文说明Autofix 审查轮次 — PR #9621提交: 各项处置
变异探针(每个新守卫/分支均有见证)
验证实际执行的命令及结果(最终提交状态):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/platform/github.ts:352 — [review] commit_id round-trip through the GitHub reader has no test; mutants dropping either spread leave the suite greenpackages/cli/src/commands/review/submit-aone.test.ts:247 — [review] Stale comment at line 569 still asserts the forced context-unavailable cap this diff removespackages/core/src/skills/bundled/review/SKILL.md:105 — [review] Aone skip bullet contradicts Step 1's setup-batch text, which runs comment-status unconditionally with pr-context
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
…ontext # 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
isPositivePrNumber alone admits two spellings whose Number() value does not round-trip to the raw string: leading zeros (007 fetches 7 but the raw string labels the heading and the prev-ledger side file, so a later 7 run reads a different side file and the round counter restarts) and digit strings above Number.MAX_SAFE_INTEGER (Number() silently rounds them, fetching a different PR than the labels announce). Add the safe-integer and no-leading-zero conjuncts — matching fetch-pr's [1-9]\d* rule — so every admitted input satisfies String(Number(x)) === x. Also pin the witnesses the round-2 review names: the commit_id round-trip through the GitHub reader and toRawReview into the persisted side file (both spreads were unwitnessed), the stale force-applies comment in submit-aone.test.ts the cap removal outdates, and the setup batch's Aone carve-out for the unbacked comment-status call.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9621Base-conflict resolution (
|
|
@qwen-code /resolve |
# Conflicts: # docs/users/features/code-review.md
|
@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: 137 passed · 0 failed · 137 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:137 通过 · 0 失败 · 137 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9621 Deep Verification (follow-up round) —
|
| # | Finding (previous round) | Severity | Status at head 2ead284 |
|---|---|---|---|
| 1 | The legacy suggestion-summary exclusion wiring is unpinned — reverting issue = allIssue.filter(!isLegacySuggestionSummary(…)) to issue = allIssue survives the whole suite; the predicate itself is pinned |
Low (pre-existing coverage gap) | stands — re-measured, not diffed: mutant M6 applied at the new head survives 288/288 (the named three files) exactly as it survived 249/249 at 0e39252. Still pre-existing (the wiring predates this PR and is carried over unchanged); still not a merge condition. The suggested follow-up (one test running a marker-bearing issue comment through the renderer) is unchanged. |
All other previous-round measurements (Aone A/B, GitHub byte-identity, pr_number matrix, gates, mutation control) were re-run at the new head — the base tip moved f1d05b7 → 2a99e84 and the head moved 0e39252 → 2ead284, so no measurement's input closure was identical and nothing was carried forward on the shortcut.
Central claim and A/B evidence
Central claim: review pr-context gains a working Aone Code backing via the platform reader (a1 CLI) — on base the subcommand is GitHub-only and dies on an Aone target — while GitHub output stays byte-identical.
Both arms are real builds run end-to-end: head = the CI-built dist at HEAD; base = HEAD^1 worktree with only packages/cli recompiled (tsc --build --noCheck, 30 s, plus the repo's own copy_files.js). The external peers (gh, a1) are fake executables on PATH answering over real execFileSync child processes — the unit under test (transport, platform reader, renderer) runs unmocked; every peer logs each invocation, so "never called" and call-order claims are counted facts. Witness: 01-ab-aone-base-vs-head.png.
Cell table — Aone target (--host gitlab.alibaba-inc.com, MR coords base-biz/dataworks-base 29408570)
| Cell | Environment | Observable oracle | Result |
|---|---|---|---|
| base | fake gh emulating the Aone host's HTML answer (the production "Before" wire shape); fake a1 present and answering |
exit ≠ 0; parse failure at '<' (HTML where JSON expected); no output file; gh invoked ≥2×; a1 invoked 0× |
failed as predicted (assertion passed) |
| head | same PATH; ambient GH_HOST=gh-ambient.example.com planted |
exit 0; title/author/state rendered; **Diff:** not reported by the platform; HEAD SHA is the trimmed padded sourceBranch; draft excluded; resolved-only root (id 201, exists only in the --resolved listing) reaches the rendered file; human blocker in the re-check section, ledger-carrier summary NOT in that section and its machine JSON nowhere in the file; side file carries recovered round 2; every refetch (≥2, non-vacuous) carries --pr 29408570 + --host gitlab.alibaba-inc.com; ambient GH_HOST absent; a1 = 1× mr view + 2× comment list (one --resolved, both --sort asc, default first) + whoami; gh 0× |
all green |
The base cell reproduces the wire shape of the production failure (HTML answered where JSON is expected), not the trigger (a real gh against gitlab.alibaba-inc.com needs credentials this sandbox does not have).
Delta cells this round (commits since the previous report)
| Claim (commit) | Transport cell | Oracle | Result |
|---|---|---|---|
| Resolved comments unioned into the bundle (9c238b1) | head, fixture where id 201 exists ONLY in the --resolved listing |
id 201 rendered in the context file; exactly one --resolved call, after the default listing |
green (also M1 below) |
| Envelope fail-closed on EITHER listing (d8575e3 + 9c238b1) | two cells: exit-0 a1.error/v1 object from the default / from the --resolved listing |
exit ≠ 0; a1 mr comment list returned an unexpected shape: <envelope message>; no output file; no untagged TypeError |
green both (also M2) |
| Context head through the ONE normalization (c02eb91) | head, sourceBranch: ' deadbeefcafe1234\n' |
rendered HEAD SHA is exactly deadbeefcafe1234 (no padding survives) |
green (also M3) |
| getCurrentUser seam contract (c02eb91) | three cells: whoami {} and {account: 123} with a critical-marker inline root; whoami {} without one |
with marker: exit ≠ 0, cannot determine the reviewing account (empty login) while a posted root comment carries a Qwen critical marker, no untagged throw, no output file; without marker: exit 0, context written |
green all three (also M4) |
Cell table — GitHub target (PR 9491, pinned gh fixtures on BOTH arms)
| Artifact | base | head | Verdict |
|---|---|---|---|
| context file | produced | produced | byte-identical |
qwen-review-pr-9491-prev-ledger.json side file |
produced | produced | byte-identical |
| stdout summary line (path-normalised) | (2 inline, 2 issue comments, 1 blocker(s)… |
same | identical |
gh invocation sequence (log) |
auth status, pr view, 3× paginated api, api user | same | identical |
a1 invocations |
0 | 0 | decoy clean |
Witness: 02-ab-github-byte-identity.png. Fixtures exercise the risky seams on both arms: unicode bodies, a reply thread (in_reply_to_id), a review with commit_id: null (null-vs-absent round-trip through toRawReview), a ledger-carrying review by the current user (recovery round 2 on both arms), an issue-kind refetch that stays global (no --pr) and a review-kind refetch that carries --pr 9491 — both emitted by >cap bodies so the checks are non-vacuous.
Secondary claims
pr_numberround-trip gate — re-measured at the new head:007,00,9007199254740993,0x10,1e3,5.0,1.5,0,-5,abcall refused with the contract message before any transport call (empty peer logs, no output file);42admitted, reachesa1 repo mr view 42, and the heading labels the same number fetched (44/44). Witness:03-prnum-boundary-matrix.png.- Write-path cap removal (
submit.ts) — theaoneWriteforce is gone and the state's claim rides the spread raw; mutant M8 re-addingcontextUnavailable: trueis killed by 3 tests including the PR's own bidirectional pin ("true stays true / false stays false"). The accepted tradeoff (a forged claim can now compose an APPROVE on Aone exactly as on GitHub) is stated in the description's Risk & Scope; no unnamed sibling found.
Corrections
None — the previous round's descriptions of the code checked out again at this head.
Findings
1. Low (pre-existing coverage gap, carried over and re-measured): the legacy suggestion-summary exclusion wiring is unpinned
Mutant M6 — reverting only const issue = allIssue.filter((c) => !isLegacySuggestionSummary(c.body)); to const issue = allIssue; — survives all three named test files at the new head (288/288 stay green), exactly as it survived at 0e39252. No test feeds a SUMMARY_MARKER-bearing comment through runPrContext/buildMarkdown and asserts its exclusion from the discussion sections; the predicate itself remains pinned by its own unit tests. Classification unchanged: coverage gap on pre-existing behavior — the wiring predates this PR and is carried over unchanged. Suggested follow-up (not a merge condition): one test running a marker-bearing issue comment through the renderer and asserting it stays out of the rendered sections.
Reproduce: in a scratch copy of packages/cli/src/commands/review/pr-context.ts apply that one-line revert, then cd packages/cli && npx vitest run src/commands/review/pr-context.test.ts src/commands/review/lib/platform/aone.test.ts src/commands/review/submit-aone.test.ts — all green.
2. Nit (test hygiene, newly observed): the mockContext once-queue cascades under call-count mutations
aone.test.ts's mockContext helper queues three mockReturnValueOnce answers (view, default listing, resolved listing), and the suite's beforeEach uses vi.clearAllMocks(), which does NOT reset the once-queue. On the unmutated code every queued value is consumed exactly, so nothing leaks (control 288/288 green). Under mutant M1 (the --resolved call removed) each helper-based test leaves one value unconsumed; the leaked values then shift the mock answers of LATER tests, so M1's kill count is 20 — the 2 direct witnesses (the union test and the resolved-envelope test, both red for the right reason) plus 18 cascade reds in tests as unrelated as composeUrl and mrPresubmitFacts. This is invisible in normal runs and is not a product defect; it is a debuggability hazard — a future legitimate change to the a1 call count would fail distant tests with confusing mock mismatches. Suggested follow-up (not a merge condition): vi.resetAllMocks() (or per-test mockImplementation) in the suite's beforeEach.
No other findings. Specifically checked and clean at the new head: the union's dedupe is first-writer-wins by id with the default listing first (matches the audit it mirrors); the disclosed residual (resolved REPLIES stay invisible — a1 exposes no listing that includes them) is stated in the code where the union happens; comment-status genuinely has no Aone backing (it is not among the six getPlatformReader consumers), so the skip SKILL.md now documents is real; the buildMarkdown trailing platform parameter breaks no caller (4537-test review suite green).
Not covered
- Live Aone E2E (real
a1againstgitlab.alibaba-inc.com, the description's MR 29408570 After-evidence): no platform credentials in this sandbox. The Aone cells prove the CLI-side handling against a scripted peer speaking through the real transport — the wire shape, not the trigger; they do not validate the reala1response grammar beyond the field spellings the code reads. - Live GitHub regression (the description's PR feat(review): post --comment reviews to Aone Code via the a1 CLI #9491 before/after diff): no
ghauth here. Substituted by the pinned-fixture byte-identity A/B (every input byte pinned — stronger for the extraction claim, complementary for real-payload coverage). - Per-commit attribution: the checkout is depth-2 (merge ref):
git rev-list HEAD^1..HEAD^2returns 1 while the metadata lists 18 commits — the gap is the shallow boundary, so only the aggregateHEAD^1..HEADdiff was verified. The merge itself is clean:git diff HEAD^2 HEADis empty, so the verified tree is exactly the PR head's tree. - Reviewer Test Plan step 2 and 3 (a1-authed run; real GitHub PR diff): not executable here for the credential reasons above; step 1 (the named vitest files) was executed — 288/288.
- A full model-driven
/reviewrun on Aone — out of scope by the PR's own plan. - Branch-based (non-AGit-Flow) MR drift-gate gap — pre-existing, filed as review: Aone Code — branch-based MRs break the write path (sourceBranch is not a SHA outside AGit-Flow) #9620 by the author; re-checked at diff level: the only
aone.tschanges outside the NEW functions are docstring edits, so the drift gate'ssourceBranchhandling is untouched here. - Repo-wide test suite: the gate was scoped to
packages/cli/src/commands/review(the affected surface), the changed core test (SKILL.test.ts), and workspace typecheck; other workspaces are untouched by this diff except docs. - Docs content (
docs/design/…,code-review.md,SKILL.md): checked for consistency with the code (consistent — the Aone comment-status skip they document matches the code's consumer list), not behaviorally executed.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at refs/pull/9621/merge; npm ci + npm run build pre-run at HEAD. Head arm ran the CI-built packages/cli/dist; base arm was a git worktree at HEAD^1 with only packages/cli recompiled (tsc --build --noCheck, 30 s). Control cleanliness, per the monorepo-link hazard: package.json/package-lock.json untouched, so the shared root node_modules is a clean dependency control; from inside the base worktree @qwen-code/qwen-code-core resolves to the head tree's packages/core (asserted via require.resolve), and git diff HEAD^1..HEAD -- packages/core/src excluding .md/tests is EMPTY — the cross-link carries no changed code; the base dist was further asserted to contain no getReviewContext symbol and to still call ghApiAll directly (6 sites). Harnesses drove the real CLI (node …/dist/index.js review pr-context …) with fake gh/a1 shims on PATH (CommonJS shims under a {"type":"commonjs"} package.json — the repo-root "type":"module" otherwise made extensionless #!/usr/bin/env node shims load as ESM); every shim logs each invocation, and scratch HOME/cwd per cell isolates config and the cwd-origin probe. Mutation matrix ran in a third worktree at HEAD (== PR-head tree, clean merge) with vitest executing TS source and sibling dist/node_modules symlinked from the head build (none is touched by this PR — asserted); unmutated control 288/288, eight mutants applied and restored with verified-clean tracked git status. Gates re-executed: named three files (288/288), full commands/review (4537 passed | 4 pre-existing skips, 0 added by the diff), core SKILL.test.ts (26/26), npm run typecheck clean with a planted TS2322 proven caught and removed. Evidence images 01–05 as referenced; raw per-cell logs, peer invocation logs, fixtures, and harness scripts live in this artifact directory (harness/, logs/).
— Qwen Code · sandboxed verification
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/cli/src/commands/review/lib/platform/aone.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/platform/aone.test.ts
file packages/cli/src/commands/review/pr-context.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/pr-context.test.ts
file packages/cli/src/commands/review/submit-aone.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/submit-aone.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/lib/platform/aone.test.ts: PPPPP
packages/cli/src/commands/review/pr-context.test.ts: PPPPP
packages/cli/src/commands/review/submit-aone.test.ts: PPPPP
packages/core/src/skills/bundled/review/SKILL.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
Evidence images
Harness 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 — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/platform/aone.ts:734 — [probe] getReviewContext's ?? c.body fallback arm has no fixture; the drop-arm mutant survives 95/95docs/design/2026-08-21-review-aone-pr-context.md:75 — [review] getCurrentUser seam contract says 'throws on lookup failure'; Aone swallows every whoami error and returns ''docs/users/features/code-review.md:385 — [review] parity claim omits the disclosed residual that resolved-thread replies stay invisible on Aonepackages/core/src/skills/bundled/review/SKILL.md:202 (+3 locations) — [probe] SKILL.md's new Aone claims (report-absence ruling, comment-status skip, Aone-backed paragraph) lack the revert-guard witness SKILL.test.ts gives the sibling appro…packages/cli/src/commands/review/lib/platform/aone.test.ts:701 — [probe] the --resolved listing's argv (third a1 call) is pinned by no test; the drop-flag mutant survives 95/95packages/cli/src/commands/review/lib/platform/types.ts:214 — [review] getCurrentUser docstring states the wrong call condition ('only when comments exist' vs the carriers-or-inline gate)docs/design/2026-08-13-review-platform-provider-abstraction.md:498 — [review] Phase 3b tracker bullet re-asserts 'cleanup audit' as open after it landed (#9617/#9633)docs/design/2026-08-21-review-aone-pr-context.md:130 — [probe] the 'suite passing unmodified' no-regression claim omits the registry pin the suite's green depends on
Convergence: round 8 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1. Findings keep coming back to the same files: packages/cli/src/commands/review/lib/platform/aone.ts (findings in round 7; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
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 — did not converge within the reverse-audit round cap of 5。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 8 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条。发现反复回到同一批文件:packages/cli/src/commands/review/lib/platform/aone.ts(第 7 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有锚点,因此下一次评审将重读整个 diff——并会一直如此,直到某一轮的标记重新带上锚点。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)
— qwen3.8-max via Qwen Code /review (v0.21.15)
…mentBody getCommentBody queried only the default comment list while the context bundle it serves refetches for unions in resolved comments — a resolved id named by a truncation note threw "not found" every time, and an exit-0 a1.error/v1 envelope threw an untagged TypeError that lost the actionable message. Extract the shape-checked default+resolved union helper and read both sites through it; witness tests mutant-checked.
…on size #9677 shrank qwen-autofix.yml from 431526 to 397656 bytes (prose moved to the design record) but recorded the baseline at 392111, 5545 below the file's own post-change size, so the first PR to run the ratchet tripped it. This branch introduces zero growth to the file (byte-identical to main); the bump aligns the baseline with reality. No workflow content changes.
|
@qwen-code /takeover from 10 |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This window's round counter starts at 10 (the rounds this PR spent in review before takeover), so the Critical-only brake engages after 0 more change-producing round(s) instead of a full fresh 5. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本窗口轮次计数从 10 起算(即本 PR 托管前已进行的评审轮数),因此再经过 0 个产生改动的轮次即进入 Critical-only,而非重新计满 5 轮。移除 |
…ontext # Conflicts: # docs/design/2026-08-13-review-platform-provider-abstraction.md # packages/core/src/skills/bundled/review/SKILL.md
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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): "agent reverse-audit (round 4)": live-Aone resolvability of path-less notes (whether closed: 1 can apply to a global comment) could not be settled — no authenticated a1 CLI in this environmen….
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
docs/design/2026-08-21-review-aone-pr-context.md:234 (+3 locations) — [review] Design-doc tracker claims contradicted by landed work (3…docs/users/features/code-review.md:385 — [review] User-doc parity claim omits the resolved-replies residualpackages/cli/src/commands/review/lib/platform/aone.test.ts:607 — [probe] The union's --resolved leg has no mutation witnesspackages/cli/src/commands/review/lib/platform/aone.test.ts:693 — [probe] Draft-skip test pins only the discussion half; the carrier…packages/cli/src/commands/review/lib/platform/aone.ts:806 — [review] getCurrentUser's catch-all contradicts the seam contract…packages/cli/src/commands/review/pr-context.test.ts:3387 — [probe] The relocated platform.ensureAuthenticated() gate has zero…packages/cli/src/commands/review/pr-context.ts:2324 — [probe] pr-context receipt structurally reports '0/0 review…packages/cli/src/commands/review/submit-aone.test.ts:411 — [review] Malformed contextUnavailable claim has no witness on the…packages/core/src/skills/bundled/review/SKILL.md:187 — [review] 'two-call batch' count contradicts the resume 'fourth…packages/core/src/skills/bundled/review/SKILL.test.ts:845 (+5 locations) — [review] SKILL.md's new/deleted Aone contract text lacks…
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.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 4)":live-Aone resolvability of path-less notes (whether closed: 1 can apply to a global comment) could not be settled — no authenticated a1 CLI in this environmen…。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 10 条(原文未翻译,列表见上方英文部分)。
机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有锚点,因此下一次评审将重读整个 diff——并会一直如此,直到某一轮的标记重新带上锚点。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
doudouOUC
left a comment
There was a problem hiding this comment.
Round 10 review
Reviewed commit 61f9c208a1e (head of feat/review-aone-pr-context).
Verdict: COMMENT — no new findings.
Context
This PR is mature (9 prior rounds). Round 9 posted 0 findings. The previous CHANGES_REQUESTED (round 8, R8-1 Critical about getCommentBody serving a narrower surface than getReviewContext) is addressed by routing getCommentBody through the same aoneAllComments union that getReviewContext uses.
Changes reviewed (14 files, +1722/-140)
- types.ts — Adds
ReviewContext,ReviewContextComment,ReviewContextVerdictinterfaces andgetReviewContext()/getCurrentUser()toReviewPlatformReader. - github.ts — Extraction of existing
pr view+ paginatedghApiAllcalls intogetReviewContext();getCurrentUser()passthrough. Byte-identical output by design. - aone.ts — New
getReviewContext()overmr view+comment list(default union--resolved, deduped by id);getCurrentUser()viaa1 auth whoami. Channel split bypathpresence, draft skip, carrier shaping from thread-level comments. - pr-context.ts — Routes through
getPlatformReader({host})instead of direct gh. NewisIssueBlocker()to prevent pipeline-owned summaries from self-promoting into the blocker section.--prbaking for every Aone refetch command.--hostpolicy: only explicit flag bakes on Aone (never ambient GH_HOST).PrMetadatastats become optional (absent on Aone). - submit.ts — Removes the forced
contextUnavailable: truefor Aone (D5). The state claim now passes through raw on both platforms. - SKILL.md / docs — Updated Aone paragraph: pr-context is backed, approve fires when context was read, Agent 0 runs,
comment-statusstays skipped. - Tests — Comprehensive new tests in aone.test.ts (channel split, draft skip, carrier shaping, author fallbacks, resolved union, envelope guarding), pr-context.test.ts (Aone routing,
--pr/--hostbaking, ledger recovery from comments,isIssueBlocker), submit-aone.test.ts (contextUnavailable claim unchanged in both directions).
Test results
pr-context.test.ts: 65/65 passedaone.test.ts: 73/73 passedsubmit-aone.test.ts: Could not run from the worktree (missing dist artifacts in fresh checkout). The main checkout's version lacks the PR's changes, so the 27 failures are expected pre-existing mismatch.
No new findings
The code is well-structured, the design decisions (D1-D7) are faithfully implemented, and the test coverage is thorough. The deferred items from prior rounds remain standing but are not blockers for this round.
--- qwen3.8-max via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix address-review round — no code changes neededCritical-only mode is active for this window. Round 10 reported no new findings, and every remaining actionable inline item was a fix claimed by a prior commit. This round re-verified each claimed fix against the live head ( Automated findings re-verified as fixed
Maintainer confirmation replies re-verifiedAll sixteen "Fixed" replies were re-checked against the current code and tests: Left unresolved
VerificationCommands actually run this round (no code was committed; the tree is unchanged at
中文说明Autofix address-review 轮次 —— 无需代码改动本窗口已启用仅处理 Critical 的模式。第 10 轮评审报告无新发现,剩余的全部可执行行内条目均为此前提交所声称的修复。本轮对每一项声称的修复在最新 head( 已重新验证为修复完成的自动评审发现
已重新验证的维护者确认回复全部十六条 "Fixed" 回复均已在当前代码与测试中复核: 保持未解决
验证本轮实际运行的命令(未提交任何代码;工作树保持在
Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 10 by
中文说明已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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: 74 passed · 0 failed · 74 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:74 通过 · 0 失败 · 74 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9621 Deep Verification (follow-up round) —
|
| # | Finding (previous round) | Severity | Status at head 61f9c208 |
|---|---|---|---|
| 1 | The legacy suggestion-summary exclusion wiring is unpinned — reverting issue = allIssue.filter(!isLegacySuggestionSummary(…)) to issue = allIssue survives the whole suite |
Low (pre-existing coverage gap) | stands — re-measured, not diffed: mutant M5 applied at the new head survives 332/332 (the named three files) exactly as it survived 288/288 and 249/249 in earlier rounds. Still pre-existing (the wiring predates this PR and is carried over unchanged); still not a merge condition. A same-file positive control (FULL_BODY_CAP 8000→80 in pr-context.ts) killed 10 tests, so the survivor is a genuine coverage gap, not a harness blind spot. Agreed deferral. |
| 2 | mockContext once-queue cascades under call-count mutations (vi.clearAllMocks() does not reset mockReturnValueOnce queues) |
Nit (test hygiene) | stands — re-measured with fresh quantitative evidence: mutant M1 (removing the union from getCommentBody, a pure call-count change) kills 4 tests — 2 direct (the resolved-refetch witness red for the right reason; the envelope test red because the mutation consumes one fewer queued answer) and 2 cascade reds in unrelated getPrMeta tests ("maps mr view onto PrMeta", "trims a padded sourceBranch") that fail only because leaked queue values shift their mock answers. Invisible in normal (unmutated) runs; still a debuggability hazard, not a product defect. Agreed deferral; the suggested follow-up (vi.resetAllMocks() in the suite's beforeEach) is unchanged. |
All other previous-round measurements (Aone A/B, GitHub byte-identity, pr_number matrix, whoami fail-closed cells, gates, cap removal) were re-run at the new head — both the base tip and the head moved, so no measurement's input closure was identical and nothing was carried forward on the shortcut.
Central claim and A/B evidence
Central claim: review pr-context gains a working Aone Code backing via the platform reader (a1 CLI) — on base the subcommand is GitHub-only and dies on an Aone target — while GitHub output stays byte-identical.
Both arms are real builds run end-to-end: head = the CI-built dist at HEAD; base = HEAD^1 worktree with only packages/cli recompiled (tsc --build, ~70 s, plus the repo's copy_files.js and generated git-commit.ts). The external peers (gh, a1) are fake executables on PATH answering over real execFileSync child processes — the unit under test (transport, platform reader, renderer) runs unmocked; every peer logs each invocation, so "never called" and call-order claims are counted facts. Witness: 01-ab-transport-matrix-base-vs-head.png (all 49 transport assertions as printed).
Cell table — Aone target (--host gitlab.alibaba-inc.com, MR coords base-biz/dataworks-base 29408570)
| Cell | Environment | Observable oracle | Result |
|---|---|---|---|
| base | fake gh emulating the Aone host's HTML answer (the production "Before" wire shape); fake a1 present and answering |
exit ≠ 0; invalid character '<' looking for beginning of value; no output file; gh invoked 2×; a1 invoked 0× |
failed as predicted (assertion passed) |
| head | same PATH; ambient GH_HOST=gh-ambient.example.com planted |
exit 0; metadata rendered; padded sourceBranch (' deadbeefcafe1234\n') rendered trimmed; degraded Diff line; resolved-only root (id 201) in the file; human blocker in re-check; ledger carrier NOT promoted and its machine JSON nowhere; draft excluded; legacy summary excluded; side file recovered round 2; every refetch (2, non-vacuous — capped blocker + capped issue comment) bakes --pr 29408570 + --host gitlab.alibaba-inc.com; ambient GH_HOST absent; a1 = 1× version + 2× whoami + 1× mr view + 2× comment list (default before --resolved, both --sort asc); gh 0× |
all green |
The base cell reproduces the wire shape of the production failure (HTML answered where JSON is expected), not the trigger (a real gh against gitlab.alibaba-inc.com needs credentials this sandbox does not have).
Cell table — GitHub target (PR 9491, pinned gh fixtures on BOTH arms)
| Artifact | base | head | Verdict |
|---|---|---|---|
| context file | produced | produced | byte-identical (20,595 chars each) |
out/qwen-review-pr-9491-prev-ledger.json side file |
produced | produced | byte-identical |
| stdout summary line | (3 inline, 2 issue comments, 1 blocker(s)… 3/3 review summaries |
same | identical |
gh invocation sequence (log) |
auth status, pr view, 3× paginated api, api user | same | identical (6 calls each) |
a1 invocations |
0 | 0 | decoy clean |
Fixtures exercise the risky seams on both arms: unicode bodies, an anchored reply thread (in_reply_to_id + the root's path/line, the realistic GitHub shape — see Findings note), a review with commit_id: null, a ledger-carrying review by the current user (round 2 recovered on both arms), and cap-exceeding bodies emitting all three refetch kinds — review-kind bakes --pr 9491, inline/issue stay global, no host baked (all asserted, non-vacuous).
Delta cells this round (commits since the previous report)
| Claim (commit) | Cell | Oracle | Result |
|---|---|---|---|
getCommentBody serves the resolved-inclusive union (6cc7476) |
A/B, fixture where id 201 exists ONLY in the --resolved listing |
BASE: exit 1, comment 201 not found in MR 29408570 (default-only gap). HEAD: exit 0, stdout exactly the body; union queried default THEN --resolved |
green both — true A/B flip |
Envelope fail-closed in getCommentBody (6cc7476) |
A/B, exit-0 a1.error/v1 object as the listing answer |
BASE: untagged (comments ?? []).find is not a function, envelope message lost, exit 2 (the handler's instanceof TypeError usage class — a correct call reported as "repair the invocation"). HEAD: exit 1, a1 mr comment list returned an unexpected shape: backend timeout — …, no TypeError |
green both |
Envelope fail-closed in getReviewContext (d8575e3/9c238b1, shared helper) |
head, envelope on the default listing / on the --resolved listing |
exit ≠ 0, tagged message, no output file — from BOTH sites | green |
Aone comment-body requires --pr for every kind |
head, no --pr |
exit 2, per-MR message, zero a1 invocations (pre-auth usage guard) | green |
| Size-baseline correction (99fe7df) | ratchet A/B, below | base trips / head passes | green |
Secondary claim — size-baseline ratchet (99fe7df0)
Witness: 03-ratchet-baseline-ab.png. The repo's own gate script, executed live on both arms:
| Arm | Entry | Actual size | Script result | Vitest mirror |
|---|---|---|---|---|
base (HEAD^1) |
392111 | 397656 | exit 1 — grew to 397656 bytes, 5545 over its recorded 392111 (allowance 4096) |
red — × qwen-autofix.yml is within its baseline allowance |
head (HEAD) |
397656 | 397656 | exit 0 — ✅ every workflow file … within 4096 bytes of its recorded baseline |
green — 181/181 |
The commit's factual claims all held: the file was already 397656 bytes at the base tip (this branch introduces zero growth — the PR diff does not touch qwen-autofix.yml), the old entry sat 5545 bytes below the file's own size, and the corrected entry equals the actual size exactly. The base-tip red means the first PR to run either gate after #9677 would have tripped — this commit repairs a live breakage, not a cosmetic drift.
Secondary claim — pr_number round-trip gate (carried, re-measured)
007, 00, 9007199254740993, 0x10, 1e3, 5.0, 1.5, 0, -5, abc — all ten refused with the contract message before any transport call (peer logs empty, no output file); 42 admitted, reaches a1 repo mr view 42, and the heading labels the same number fetched.
Write-path cap removal (submit.ts, carried)
Mutant M4 re-adding contextUnavailable: true to the compose call is killed by 3 tests, including the PR's own bidirectional pin ("the contextUnavailable claim crosses the Aone seam unchanged, in BOTH directions"). The accepted tradeoff (a forged claim can now compose an APPROVE on Aone exactly as on GitHub) is stated in the description's Risk & Scope; no unnamed sibling found — the forced value had exactly one consumer (the removed aoneWrite parameter), and compose-review's boundary still refuses a malformed non-boolean.
Corrections
None — the previous rounds' descriptions of the code checked out again at this head. SKILL.md's new claims were re-checked against the code: pr-context/comment-body are among the six getPlatformReader consumers; comment-status has no reader (the documented Aone skip is real); the approval-gating prose matches the cap removal.
Findings
1. Low (pre-existing coverage gap, carried over and re-measured): the legacy suggestion-summary exclusion wiring is unpinned
Mutant M5 — reverting only const issue = allIssue.filter((c) => !isLegacySuggestionSummary(c.body)); to const issue = allIssue; — survives all three named test files at the new head (332/332 stay green). No test feeds a SUMMARY_MARKER-bearing comment through runPrContext/buildMarkdown and asserts its exclusion from the discussion sections; the predicate itself remains pinned by its own unit tests. Classification unchanged: coverage gap on pre-existing behavior — the wiring predates this PR and is carried over unchanged; not a merge condition. Suggested follow-up (unchanged): one test running a marker-bearing issue comment through the renderer and asserting it stays out of the rendered sections.
Reproduce: in a scratch copy of packages/cli/src/commands/review/pr-context.ts apply that one-line revert, then cd packages/cli && npx vitest run src/commands/review/pr-context.test.ts src/commands/review/lib/platform/aone.test.ts src/commands/review/submit-aone.test.ts — all green.
2. Nit (test hygiene, carried over and re-measured): the mockContext once-queue cascades under call-count mutations
Re-measured with this round's M1 mutant: removing the union from getCommentBody (one fewer a1Json call per helper-driven test) kills 4 tests — the resolved-refetch witness (red for the right reason), the envelope test (red because the mutated code consumes one fewer queued answer), and two getPrMeta tests that have nothing to do with the mutation ("maps mr view onto PrMeta", "trims a padded sourceBranch"), failed purely by leaked mockReturnValueOnce values shifting their mocks. The mechanism is visible in the suite source: mockContext queues three once-values and every beforeEach uses vi.clearAllMocks(), which does not reset once-queues. Invisible in normal runs; a future legitimate change to the a1 call count would fail distant tests with confusing mock mismatches. Not a merge condition. Suggested follow-up (unchanged): vi.resetAllMocks() in the suite's beforeEach.
Open question (NOT verified here — not a finding): the envelope guard's sibling call site
The shape-checked helper covers mr comment list, where the exit-0 a1.error/v1 object was measured. One other a1Json site in the same provider still coalesces with ?? [] before .map — getClosingIssues on mr workitem list — and would throw the same untagged TypeError if that endpoint can answer with the same envelope shape. Whether it can was not established here (needs a real a1/backend outage to observe); flagged only so the next round or the author can check. Everything else in the provider either guards (comment list), tolerates (a1JsonOnce on writes), or fails with a tagged message (mrView's no-mergeRequest refusal).
Not covered
- Live Aone E2E (real
a1againstgitlab.alibaba-inc.com, the description's MR 29408570 After-evidence): no platform credentials in this sandbox. The Aone cells prove CLI-side handling against a scripted peer speaking through the real transport — the wire shape, not the trigger; they do not validate the reala1response grammar beyond the field spellings the code reads. - Live GitHub regression (the description's PR feat(review): post --comment reviews to Aone Code via the a1 CLI #9491 before/after diff): no
ghauth here. Substituted by the pinned-fixture byte-identity A/B (every input byte pinned — stronger for the extraction claim, complementary for real-payload coverage). - Per-commit attribution: the checkout is depth-2 (merge ref):
git rev-list HEAD^1..HEAD^2returns 1 while the metadata lists 22 commits — the gap is the shallow boundary, so only the aggregateHEAD^1..HEADdiff was verified. The merge itself is clean (git diff HEAD^2 HEADempty). - Reviewer Test Plan steps 2 and 3 (a1-authed run; real GitHub PR diff): not executable here for the credential reasons above; step 1 (the named vitest files) was executed — 332/332.
- A full model-driven
/reviewrun on Aone — out of scope by the PR's own plan; the downstream consequences the description claims (Agent 0 unskipped, ledger read-back, approval firing) ride skill prose +submitbehavior verified only at the deterministic level (cap-removal mutant, SKILL.md/code consistency). - Branch-based (non-AGit-Flow) MR drift-gate gap (review: Aone Code — branch-based MRs break the write path (sourceBranch is not a SHA outside AGit-Flow) #9620) — pre-existing, author-declared out of scope; re-checked at diff level: this round's
aone.tsdelta is confined togetCommentBodyand the shared listing helper; the head-SHA read sites are untouched. - Repo-wide test suite: gates scoped to the affected surface —
packages/cli/src/commands/review(4647/4647 + 4 pre-existing skips, zero added by the diff), the changed core test (SKILL.test.ts 30/30), the size-mirror (181/181), and workspace typecheck; other workspaces are untouched by this diff except docs. - Docs content (
docs/design/…,code-review.md): checked for consistency with the code (consistent), not behaviorally executed. - Degenerate reply shape on GitHub: a thread reply whose anchor GitHub dropped (a
pulls/commentsentry within_reply_to_idbut nopath) would classify into the issue channel under head's path-presence re-split while base kept it in the endpoint's inline channel — the realistic shape carries the root's anchor (as used in the byte-identity fixture and the repo's transport history), so this is a hypothetical shape I could not prove GitHub emits; noted for completeness, not asserted as a defect.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at refs/pull/9621/merge; npm ci + npm run build pre-run at HEAD. Head arm ran the CI-built packages/cli/dist; base arm was a git worktree at HEAD^1 with only packages/cli recompiled (tsc --build + copy_files.js + generated git-commit.ts; the compile reported 275 TS2307s confined to test files and optional-telemetry imports whose dev-deps the CI image does not install — zero errors in any commands/review file, and the base dist's runtime behavior was proven by the cells themselves, including its predicted failure). Control cleanliness, per the monorepo-link hazard: package.json/package-lock.json untouched by the PR, so the shared root node_modules is a clean dependency control; from the base worktree @qwen-code/qwen-code-core resolves (walk-up; readlink -f node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/packages/core) into the head tree's core, and git diff HEAD^1..HEAD -- packages/core/src excluding .md/tests is EMPTY — the cross-link carries no changed production code. Harnesses drove the real CLI (node …/dist/index.js review …) with fake gh/a1 shims on PATH (bash wrappers exec-ing .cjs implementations — the repo-root "type":"module" otherwise loads extensionless node scripts as ESM); every shim logs each invocation, and scratch HOME/cwd per cell isolates config and the cwd-origin probe. Harness scripts and per-cell logs live in this artifact directory (harness/, logs/). Mutation matrix: six single-point mutants applied to the head tree one at a time, run against the three named files (unmutated control 332/332 green on the identical command), each restored and git status verified clean; positive controls landed in the mutated files (author-key swap in aone.ts killed 1; FULL_BODY_CAP shrink in pr-context.ts killed 10). Gates re-executed live under scripted expectations (witness 04-gates-live.png); typecheck liveness proven by planting a TS2322 in aone.ts (caught at aone.ts(14,7)), restoring, and re-running clean (cli + core exit 0). The ratchet harness's first revision undercounted the mirror rows because vitest's colored output interleaves ANSI escapes into the × line; the fixed harness strips them and was re-run green end-to-end — the earlier red was an instrument bug, not a PR signal. Evidence images: 01-ab-transport-matrix-base-vs-head.png, 02-mutation-matrix.png, 03-ratchet-baseline-ab.png, 04-gates-live.png.
— Qwen Code · sandboxed verification
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/cli/src/commands/review/lib/platform/aone.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/platform/aone.test.ts
file packages/cli/src/commands/review/pr-context.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/pr-context.test.ts
file packages/cli/src/commands/review/submit-aone.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/submit-aone.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/lib/platform/aone.test.ts: PPPPP
packages/cli/src/commands/review/pr-context.test.ts: PPPPP
packages/cli/src/commands/review/submit-aone.test.ts: PPPPP
packages/core/src/skills/bundled/review/SKILL.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/platform/aone.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/submit-aone.test.ts: P (exit 0)
round 5 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
Evidence images
Harness 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. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round: no action takenCritical-only mode is active for this window: the round counter reached 5 (this window was seeded at round 10 by No actionable input remained for this round:
The single item listed under No code changes were made, no commits were created, no review threads were resolved, and no comment replies were written. No base merge was performed ( 中文说明Autofix 轮次:未采取任何操作本窗口已处于仅处理 Critical 的模式:轮次计数已达 5(本窗口由 本轮没有剩余可执行输入:
未修改任何代码,未创建任何提交,未解决任何评审线程,也未写入任何评论回复。未进行 base 分支合并( Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 10 by 中文说明已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
There was a problem hiding this comment.
Independent review at 61f9c208 — all six Criticals verified fixed; the cap removal is earned; approving
The load-bearing judgement in this PR is not any single hunk — it is whether lifting the forced context-unavailable cap is now earned. That force existed because no Aone run could have read the MR's discussion, and removing it makes APPROVE reachable on Aone for the first time, with a1 repo mr approve behind it. So I reviewed it as "is the read surface complete enough to certify an absence of blockers", and checked each closed Critical against the code rather than against its reply.
The six Criticals — verified at this head
Head normalization (R5-1) — getReviewContext routes both fields through aoneHeadSha (aone.ts:787-788), so the context file can no longer carry a padded SHA while every sibling read reports the trimmed one.
Envelope shape (R6) — aoneCommentListing (aone.ts:361-388) rejects a non-array with a tagged error that carries the a1.error/v1 message. The untagged TypeError from .filter on an object is gone, and the actionable cause survives.
Resolved-comment blindness (R7-1 / R8-1) — aoneAllComments (aone.ts:407-412) unions the default and --resolved listings, deduped by id, and both consumers read through it: getReviewContext (:742) and getCommentBody (:703). That closes the asymmetry where the context file rendered a resolved id whose refetch would then throw "not found". I checked the compatibility question this raised — --resolved is not a new requirement: main already ships it at cleanup.ts:589, so no a1 floor moves.
Self-promoting summaries (R2-1) — isIssueBlocker (pr-context.ts:131-133) is carriesBlockerSignal(b) && stripLedgerMarker(b) === b, and it gates the split at :1885-1886 and the count at :2322. The docstring names its own tradeoff honestly rather than hiding it: keying on the marker means a third party embedding one demotes their own comment and nobody else's — the failure direction is confined to the embedder.
SKILL contract contradiction (R5-2) — the report-less ruling now distinguishes "no report was ever written" (lightweight, Aone) from "the command ran and failed", so it no longer contradicts the three unchanged comment-status contract sites.
Why I think the cap removal holds
Two things had to be true, and both are:
The identity gate fails closed on the shape that used to slip. pr-context.ts:2199 throws when !identityKnown && anyRootCarriesCriticalMarker(inline), and identityKnown is login !== '' — so the exit-0-with-empty-output shape is covered, not just the throwing one. The comment above it documents precisely the prior bug (the check "used to live in the catch branch only, so the empty login proceeded with me = ''"). That matters here more than usual: with the cap gone, an unresolved attribution-off Critical silently reclassifying as ordinary discussion is the path to a wrongly-reachable APPROVE.
Nothing approves vacuously off the empty verdict list. Aone returns verdicts: [] (aone.ts:791) because no review object exists there. I checked that this is a stated absence rather than a silent gap: the design doc's surface table says so, and blockers on Aone route through comments instead — ledgerCarriers is the path-absent comment set, and blocker detection is body-based (isBlockerBody / carriesBlockerSignal / isIssueBlocker). So the empty list feeds display, not gating.
The residual is disclosed in both places it should be — resolved replies remain invisible because no a1 listing includes them (aone.ts:402-405, :743, and design doc D2/D4). That is the right call for this mechanism: a reply alone never retires a blocker, so the re-check walk is unaffected.
Other checks
The new ReviewPlatformReader members are not dead switches: getReviewContext / getCurrentUser are declared in types.ts:210/217, implemented by both aone.ts:733/796 and github.ts:302/375, and called from pr-context.ts:2107/2186. Untrusted comment bodies enter under the existing PREAMBLE data-only framing, which covers the newly-added channels. No credential or token reaches the context file — the identity surface is a login string used only for local comparison. House style is clean: no any, no cross-package relative imports, tests collocated.
An independent second pass over the provider abstraction, the blocker budget accounting, and the submit path returned no Critical and no Suggestion, matching my own reading.
Where I land
Approving. Six Criticals closed and verified against the code, CI green, MERGEABLE, and the one behavioural change that carries real risk is backed by a read surface whose gaps are named rather than assumed. reviewDecision reads REVIEW_REQUIRED rather than blocked.
Not verified by me: the suites did not run locally (no node_modules in my worktree — CI covers them green), and no live Aone MR was exercised, so the a1 read path is judged from code plus its mocked tests. Integration Tests (CLI, No Sandbox) was skipped in CI and I did not cover it either.
… 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.
…ll owes Round-4 review found the same-repo pr-context failure path promising a terminus it cannot reach. The paragraph said the handling was "identical to lightweight mode: warn, continue, skip Agent 0" — but the two paths differ on the one field that decides the roster. A lightweight plan never gains the PR identity (plan-diff omits --pr/--repo when the fetch failed), so requiredAgents stops owing the roles gated on it; fetch-pr has already written that identity into a same-repo plan, so role 0 stays required and a skipped Agent 0 lands in missingRoles, exits check-coverage 3, and the capped-COMMENT terminus the same paragraph describes is unreachable. Launch them instead of skipping them — Agent 0, and 6d at high or unrecorded effort — and let each return what its brief already has for a missing context file. Both returns clear Step 3D and join unreviewedDimensions, which is the state the run is actually in. Also from the round: - 6d's "stays on the roster … so launch it" gained the effort qualifier its two sibling statements already carried; countersFrame is false at medium, so an unqualified launch spends a budget-exempt agent the tier does not owe and records a phantom unreviewed dimension. - The plan-diff rationale said Agent 0 and 6d were agents "nobody can brief". Given the identity, agent-prompt builds both with no context file on disk; it throws only when the identity is absent. Omitting --pr/--repo drops the requirement — that, not unbriefability, is why the flags stay off. - The budget paragraph enumerated three exemptions where the briefs now declare five: 6d's mandated context read is discussion-sized and prose-exec's work is recipe-sized, so neither carries a ceiling. - The chunk-agent brief carved the counter-frame audit out of chunk duties unconditionally, while countersFrame owes 6d only on a PR target at non-medium effort and isTerritoryFanOut is size-only. A medium 3B review told every chunk agent to defer an out-of-frame signal to an agent that never launched. The clause now carries the same "where the run owes it" qualifier its prose-exec sibling has, in the builder and in the SKILL.md twin, pinned by a test that goes red on the unconditional wording. The round's remaining finding — "non-medium" in the Aone skip bullet — was retired by the merge with main: #9621 backs pr-context on Aone, so the bullet that carried the claim is gone and 6d reads a real context file there.
|
Released in v0.22.2. |









What this PR does
The Aone Code read path and write path for
/reviewlanded in earlier phases, but the subcommand that fetches a target's metadata and existing discussion into the context file every review agent reads was still GitHub-only. On an Aone target it was skipped, and the skip cascaded: every Aone run was declared context-unavailable, the verdict was force-capped at Comment so the already-wired native approval could never fire, the Issue Fidelity agent never ran, and the machine ledger a previous round posted on the MR was never read back — every round re-opened as round 1.This change routes that context fetch through the platform abstraction. Aone serves it from the a1 CLI: the MR metadata, the flat comment collection split into its inline and thread channels, and the posted qwen summaries, which carry the machine-ledger markers and feed the same recovery walk GitHub uses for review bodies. The identity fail-closed policy, blocker promotion, ledger selection, and rendering all stay where they were — platform-neutral over the normalized bundle. The GitHub implementation is an extraction of the existing calls; its output is byte-identical. With the context reads backed, the write path stops forcing the context-unavailable cap and treats the run's claim exactly as it does on GitHub, and the refetch commands a context file emits carry the per-MR addressing Aone requires.
Why it's needed
AGit-Flow amend-and-re-review is the dominant loop on Aone, and today every such round reviews blind: it cannot see the discussion already on the MR, cannot recover which round it is, cannot elevate previously-posted blockers into the mandatory re-check, and can never certify a clean MR with an approval. This is the single change that lifts all four, and it is the prerequisite for the remaining Aone follow-ups (cross-round dedup, AI-comment marking) that build on a working context read.
Reviewer Test Plan
How to verify
The deterministic surface is covered by unit tests and needs no platform access:
cd packages/cli && npx vitest run src/commands/review/pr-context.test.ts src/commands/review/lib/platform/aone.test.ts src/commands/review/submit-aone.test.ts— the new cases pin the Aone channel split (path presence marks inline comments), draft skipping, ledger carriers shaped from thread comments, the degraded Diff header line, refetch commands baking--prand the host, the ambientGH_HOSTnever baking into Aone refetches, and the write path handing the context-unavailable claim through unchanged in both directions.With a1 auth and a clone of any Aone repo: run
node dist/cli.js review pr-context <mr-id> <group>/<project> --host gitlab.alibaba-inc.com --out /tmp/ctx.mdfrom inside the clone and confirm the context file carries the MR's title/author/state/description and its comments, withDiff: not reported by the platform(Aone reports no stats). Run one GitHub PR before and after the change and diff the outputs — they must be identical.Evidence (Before & After)
Before (global release, Aone target): the command dies with
invalid character '<' looking for beginning of value— it routes gh at the Aone host and chokes on the HTML answer; no context file.After (this branch, MR 29408570 of base-biz/dataworks-base): exit 0,
Wrote PR context … (0 inline, 1 issue comments, …); the context file renders the MR metadata, the degraded Diff line, and the existing AI-review comment in full with a working refetch command (comment-body <id> --kind issue --pr <mr-id> --repo … --host gitlab.alibaba-inc.com, executed and verified). GitHub regression: PR #9491's context file is byte-identical before/after (diff empty, same counts 200/33/35/120).Tested on
Environment (optional)
npm run build && npm run bundle, thennode dist/cli.js; a1 v0.1.90 against gitlab.alibaba-inc.com.Risk & Scope
sourceBranchis a branch name there and no head-SHA field exists, which the drift gate compares against a SHA; filed as review: Aone Code — branch-based MRs break the write path (sourceBranch is not a SHA outside AGit-Flow) #9620, untouched here. A full-skill Aone review run (model-driven) was not exercised; the deterministic subcommand surface was.Linked Issues
Follow-ups tracked separately: #9613 (cross-round dedup), #9614 (AI-comment marking), #9615 (removed-line anchors), #9616 (self-PR detection), #9617 (cleanup audit), #9618 (AGit-Flow incremental cache), #9619 (residual small gaps), #9620 (branch-based MR gap found during this work's E2E). Design:
docs/design/2026-08-21-review-aone-pr-context.md.中文说明
这个 PR 做了什么
/review的 Aone Code 读路径与写路径已在前期阶段落地,但"把目标元数据与既有讨论抓取为 context 文件(每个审查 agent 的阅读材料)"的子命令仍是 GitHub-only。Aone 目标上它被跳过,且跳过引发连锁:每个 Aone run 都被判为 context-unavailable,verdict 被强制封顶在 Comment,已接好的原生 approval 永远无法触发,Issue Fidelity agent 从不运行,上一轮发布在 MR 上的 machine ledger 也从不回读——每一轮都作为第 1 轮重新开始。本改动把该 context 抓取改走 platform 抽象。Aone 侧由 a1 CLI 供数:MR 元数据、按有无 path 拆分为 inline 与线程两通道的扁平评论集合,以及承载 machine-ledger marker 的已发布 qwen 总结——它们进入与 GitHub review body 相同的恢复走查。身份 fail-closed 策略、blocker 提升、ledger 选择与渲染全部留在原处,对归一化数据保持平台无关。GitHub 实现是对既有调用的原样抽取,输出逐字节一致。context 读取有了 backing 之后,写路径不再强制 context-unavailable 封顶,转而像 GitHub 一样对待 run 的声明;context 文件发出的补取命令也带上 Aone 所需的按 MR 寻址参数。
为什么需要
AGit-Flow 的 amend-再审查是 Aone 上的主导循环,而今天每一轮都是盲审:看不到 MR 上已有的讨论、恢复不出当前轮次、无法把已发布的 blocker 提升到强制复核区、也永远无法用 approval 认定干净的 MR。这是同时解除四者的唯一改动,也是其余 Aone 后续工作(跨轮去重、AI-comment 标记)的前置——它们都建立在可用的 context 读取之上。
评审者测试计划
如何验证
确定性表面由单测覆盖,无需平台访问:
cd packages/cli && npx vitest run src/commands/review/pr-context.test.ts src/commands/review/lib/platform/aone.test.ts src/commands/review/submit-aone.test.ts——新增用例钉住:Aone 通道拆分(path 存在即 inline)、草稿跳过、由线程评论塑形的 ledger carriers、降级的 Diff 头行、补取命令固定携带--pr与 host、环境 GH_HOST 永不进入 Aone 补取命令、写路径对 context-unavailable 声明双向原样透传。有 a1 认证和任一 Aone 仓库 clone 时:在 clone 内运行
node dist/cli.js review pr-context <mr-id> <group>/<project> --host gitlab.alibaba-inc.com --out /tmp/ctx.md,确认 context 文件包含 MR 的标题/作者/状态/描述及其评论,且 Diff 行为not reported by the platform(Aone 不上报统计)。改动前后各跑一个 GitHub PR 并 diff 输出——必须完全一致。前后对比证据
改动前(全局 release,Aone 目标):命令死于
invalid character '<' looking for beginning of value——gh 被路由到 Aone host,解析 HTML 应答失败;无 context 文件。改动后(本分支,base-biz/dataworks-base 的 MR 29408570):exit 0,
Wrote PR context … (0 inline, 1 issue comments, …);context 文件渲染出 MR 元数据、降级的 Diff 行,以及既有的 AI 审查评论全文与可运行的补取命令(comment-body <id> --kind issue --pr <mr-id> --repo … --host gitlab.alibaba-inc.com,已实际执行验证)。GitHub 回归:PR #9491 的 context 文件改动前后逐字节一致(diff 为空,计数同为 200/33/35/120)。测试环境
运行环境(可选)
npm run build && npm run bundle后node dist/cli.js;a1 v0.1.90,目标 gitlab.alibaba-inc.com。风险与范围
sourceBranch是分支名且不存在 head SHA 字段,而 drift gate 用它对比 SHA;已建 review: Aone Code — branch-based MRs break the write path (sourceBranch is not a SHA outside AGit-Flow) #9620,本 PR 不动。未跑模型驱动的完整 skill Aone 审查;已验证的是确定性子命令表面。关联 Issue
后续工作分别跟踪:#9613(跨轮去重)、#9614(AI-comment 标记)、#9615(删除行锚点)、#9616(self-PR 检测)、#9617(cleanup 审计)、#9618(AGit-Flow 增量缓存)、#9619(其余小项)、#9620(本次 E2E 中发现的分支型 MR 缺口)。设计文档:
docs/design/2026-08-21-review-aone-pr-context.md。