feat(serve): make Aone a first-class platform for session PR bindings - #10287
Conversation
Session PR bindings were GitHub-only on their automatic paths: on Aone workspaces the backfill fabricated non-existent `<origin>/pull/<N>` URLs and the refresh sweep never updated a binding's state. Detect each workspace's PR platform from its origin and add an a1-backed provider: branch-to-MR mapping pages `a1 repo mr list`, every bound URL comes from `a1 repo mr view` (Aone links are never assembled), and the sweep refreshes states one view per unique pending MR. Bindings already persisted in the fabricated shape are detected and repaired in place with the real URL and state, keeping their createdAt. The GitHub path is unchanged, and the same-PR identity guard fails closed on Aone so a foreign same-numbered binding can never be trimmed and flipped.
|
|
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献!
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewReviewed the diff against the base at What I verified while reading:
Tests are the injected-backend kind but substantive: 13 cases across the three files — detection against real throwaway git repos (scp-style, nested groups, family predicate, non-Aone, unreadable origin, nested cwd), global-id-vs-iid parsing, the 25-call cap on both paths, degraded list/view modes, highest-number-wins on a reused branch, foreign same-numbered binding under cap pressure, and legacy repair with Nothing blocking found — no correctness bugs, no security holes, no convention violations. One acknowledged tradeoff, already documented in the design doc: a pathological backfill run can serialize up to ~6 list + 25 view calls at a 20s timeout each — acceptable for a manually triggered maintenance route, and the refresh sweep's The Aone backfill flow, for orientation: sequenceDiagram
participant P1 as Client
participant P2 as Backfill route
participant P3 as aone-mrs backend
participant P4 as a1 CLI
participant P5 as PR sidecar
P1->>P2: POST backfill-prs
P2->>P3: resolveAoneWorkspaceRepo, workspace origin
P3-->>P2: repoPath, or undefined for GitHub
P2->>P3: list opened then merged, 3 pages each
P3->>P4: a1 repo mr list
P4-->>P3: entries with id, sourceBranch, state
Note over P2: branch mapping - highest number wins, default branch excluded
P2->>P3: view each new or legacy number, cap 25 per run
P3->>P4: a1 repo mr view id
P4-->>P3: detailUrl plus state
P2->>P5: commit bindings, repair legacy pull URLs in place
TestingThis is an unattended CI run — the PR's code was not built or executed here; the evidence below is the PR's own CI, read through the API. The main unit suite and the Serve A/B job were still running at review time; the table updates in place once CI settles. Not verified here: anything requiring a real Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The Sandboxed verification would settle the one claim 2b cannot: 中文说明代码审查先说独立基线:这个问题我会这么做——按工作区 origin 判定平台、复用 review 子系统里依赖轻的 Aone 原语、带页上限地列 阅读中核实过的点:
测试是注入后端式的,但足够扎实:三个文件共 13 例——用真实临时 git 仓库做检测(scp 式、嵌套分组、family 谓词、非 Aone、origin 不可读、嵌套 cwd)、global id 与 iid 的解析、两条路径上的 25 次上限、list/view 降级、复用分支取最大号、上限压力下的外部同号绑定保护、以及保留 未发现阻塞项——无正确性 bug、无安全问题、无规范违反。一个已记录的权衡:极端情况下一次回填可能串行 ~6 次 list + 25 次 view、每次 20s 超时——对手动触发的维护路由可接受,且刷新轮询有 测试本次为无人值守 CI 运行——未在此构建或执行 PR 代码;以上证据来自 PR 自身 CI(经 API 读取)。审查时主单测套件与 Serve A/B 仍在运行,表格会在 CI 结束后原地更新。此处未验证:任何需要真实 沙箱验证可以补齐 2b 补不上的一条: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage I can verify; the one open item is epistemic, not a code defect (the Aone side of the behavior rests on the author's real- Stepping back: this is what a well-formed feature PR looks like. The problem is observed and specific — dead links and permanently stale badges in the session list on any Aone workspace — not a theoretical hardening, and the before/after E2E in the description shows the exact fabricated URL the old code produced. The direction is a completion of an existing commitment: the review subsystem is already first-class on Aone, and session PR bindings were the corner still pretending GitHub is the only platform. The implementation matches my independent proposal nearly point-for-point, and where it diverges it diverges upward: the legacy fabricated-URL repair (which I'd have filed as a follow-up and would have regretted, since un-repaired entries burn a view call per sweep forever), and the deliberate async transport split with a rationale that checks out against the actual If I'm maintaining this in six months, the design doc, the platform-facts table, and the why-comments make it a thank-you, not a curse. The 13 injected-backend cases cover the paths that matter, including the ugly ones (cap pressure with a foreign same-numbered binding, view failure mid-repair). Reservations, honestly named: the live-platform half — that real CI: the main unit suite ( 中文说明置信度:4/5 —— 我能验证的每一关都干净;唯一未决项是认知层面的,不是代码缺陷(Aone 侧行为依赖作者真实 退一步看:这是一个形态良好的功能 PR。问题是观测到且具体的——任何 Aone 工作区的会话列表里都是死链与永久过期的徽章——不是理论性加固;描述里的 before/after E2E 展示了旧代码产出的确切虚构 URL。方向是既有承诺的收尾:review 子系统在 Aone 上已是一等公民,会话 PR 绑定是仍在假装 GitHub 是唯一平台的角落。 实现与我的独立方案逐点吻合,分歧处都是向上的:旧版虚构 URL 修复(我原本会拆成后续 PR,之后必然后悔——不修复的条目每轮刷新都白烧一次 view 调用),以及刻意的异步传输拆分,其理由对照 六个月后维护这段代码:设计文档、平台事实表、讲"为什么"的注释让人心存感激而非咒骂。13 个注入后端用例覆盖了要紧的路径,包括难看的那些(上限压力下的外部同号绑定、修复中途 view 失败)。 坦率说出保留意见:真实平台那一半——真实 CI:审查时主单测套件( — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
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. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): chunk 4: execute packages/cli/src/serve/server/aone-mrs.test.ts to confirm it passes — the review worktree has no node_modules and the install+build exceeds the tool…; chunk 6: executed session-pr-refresh.test.ts in packages/cli to confirm the new tests pass — the review worktree has no node_modules (vitest startup failed on miss….
中文说明
未探索到全部深度(达到工具调用预算):chunk 4:execute packages/cli/src/serve/server/aone-mrs.test.ts to confirm it passes — the review worktree has no node_modules and the install+build exceeds the tool…;chunk 6:executed session-pr-refresh.test.ts in packages/cli to confirm the new tests pass — the review worktree has no node_modules (vitest startup failed on miss…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@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 冲突,直到移除标签或达到轮次上限。移除 |
…er (#10287) Address round-1 review findings: - Re-attest existing planned numbers through mr view on Aone so they pass the closed same-PR identity guard; without it every entry of a full sidecar counted foreign and cap eviction stalled permanently (Critical). - Run the legacy fabricated-binding repair before the empty-numbers skip so sessions whose branches fell out of the list window stay repairable. - Stop memoizing a FAILED a1 --version probe; the next read re-probes so the version floor rules again after a transient probe failure. - Add an a1 exec seam (setA1ExecForTest) and unit tests pinning the parsed-shape-over-exit-code error contract, ENOENT mapping, version floor, and the short-page paging break. - Witness the Aone fail-closed default-branch guard and the unplanned repair-commit path; order the reused-head-branch fixture descending to match a1's newest-first listing and kill the last-wins mutant. - Correct the documented viewAoneMergeRequest result shape.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review round response — PR #10287 (feedback round 1)Commit: DispositionsResolved in code (8):
Deferred to the next round (4) — batch bounded at ~8 findings with the
Changed files
Mutation probes (every new guard witnessed)
Verification
中文说明评审轮次回复 — PR #10287(反馈第 1 轮)提交: 处置结果已在代码中解决(8 项):
延迟到下一轮(4 项)——批次按最多约 8 个发现项封顶、Critical 优先;每个延迟项在各自线程里有说明原因的回复:
变更文件
变异探针(每个新守卫都有见证)
验证
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.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/session-pr-backfill.ts:621 — [probe] fail-closed guard's missing-attestation case unpinnedpackages/cli/src/serve/routes/session-pr-backfill.ts:594 — [probe] commit-time repair re-check unpinnedpackages/cli/src/serve/routes/session-pr-backfill.ts:365 — [probe] viewCache cross-candidate dedupe unpinnedpackages/cli/src/serve/routes/session-pr-backfill.ts:402 — [probe] platform: 'github' wire value unpinnedpackages/cli/src/serve/server/aone-mrs.ts:349 — [probe] list paging continuation unpinnedpackages/cli/src/serve/server/aone-mrs.ts:98 — [probe] dead gitRoot field in the detection APIpackages/cli/src/serve/server/aone-mrs.ts:400 — [probe] defaultAoneMrBackend adapter untesteddocs/design/2026-08-27-session-pr-aone-provider.md:82 — [review] design doc asserts the family-wildcard safety the confirmed Critical falsifiespackages/cli/src/serve/routes/session-pr-backfill.test.ts:739 — [probe] view-attestation-wins state semantics unpinned
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 9 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@qwen-code /takeover |
|
🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds no longer count toward the cap); management continues. 中文说明🔄 已重新武装:轮次计数开启新窗口(此前轮次不再计入上限),托管继续。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the pre-trim resolution budget leak (session-pr-backfill.ts:543-548) — folded into the carried R2-6 re-post, which pins the same steady-state defect
- list paging continuation beyond page 1 untested (aone-mrs.ts) — already deferred in round 2's convergence list (review 5044930006)
- fail-closed identity guard's missing-attestation case unpinned (session-pr-backfill.ts:621) — already deferred in round 2's convergence list
- commit-time repair re-check unpinned (session-pr-backfill.ts:594) — already deferred in round 2's convergence list
- design doc family-wildcard safety claim (docs/design:82) — already deferred in round 2's convergence list; tied to confirmed R2-1
Not explored to full depth (tool budget reached): chunk 5: executed npx vitest run src/serve/server/aone-mrs.test.ts in packages/cli — the review worktree has no node_modules and no built workspace dist/ , and a ….
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/session-pr-backfill.ts:319 — [review] shared try across opened+merged mr list discards partial success (aoneAvailable: false for the whole run)packages/cli/src/serve/server/session-pr-refresh.ts:190 — [review] sweep spends capped view budget on never-applicable fabricated legacy bindingspackages/cli/src/serve/server/aone-mrs.ts:330 — [review] accepted MRs invisible to branch mapping (list window omits the accepted bucket)packages/cli/src/serve/routes/session-pr-backfill.test.ts:619 — [review] no witness that legacy repair still runs when mr list fails
中文说明
本轮确认的 5 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):chunk 5:executed npx vitest run src/serve/server/aone-mrs.test.ts in packages/cli — the review worktree has no node_modules and no built workspace dist/ , and a …。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…10287) Address review rounds 1-2: - Detect on the canonical host pair only: the family wildcard displaced GitHub Enterprise workspaces (*.alibaba-inc.com also names GHE hosts) onto a1, freezing state where gh served before and risking an unrelated same-path repo's MR. GHE-shaped origins now keep the GitHub path. - Replace mr-view re-attestation of existing planned numbers with an exact detailUrl-shape identity check (isAoneDetailUrlForRepo): a full sidecar re-planned entries stay trimmable without spending the shared view budget every run, which the re-attestation leaked in steady state. - Refresh sweep: drop numbers whose stored URLs miss this repo detailUrl shape (foreign/legacy bindings can never match), rotate the capped view window across sweeps so more-than-25 pending numbers are not starved, and bound the view loop with a 60s aggregate deadline so hung a1 calls cannot stall the whole timer past its interval. - Give the a1 --version probe its own timeout; export the exec constants. - Pin the rest: argv/options through the exec seam, view success path, gitEnv sanitization, reused-branch rule in both arrival orders, two-run steady state, GHE routing, and the new sweep behaviors.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped at the 5-round plan cap with round 5 still reporting (1 new finding); rounds beyond the cap were not run.
Not explored to full depth (tool budget reached): chunk 5: none — one check (actually executing the test file) was deliberately not run because the worktree has no installed dependencies; it was replaced by full manual ….
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/server/aone-mrs.ts:387 — [probe] no test fetches page 2 of listAoneMergeRequests (paging continuation unpinned) — recorded in rounds 2-3, deferred again under the code-age rulepackages/cli/src/serve/server/aone-mrs.ts:327 — [probe] positive version-probe memoization unpinned — deferred under the code-age ruledocs/design/2026-08-27-session-pr-aone-provider.md:135 — [review] accepted-state MRs invisible to branch mapping (parity gap) — recorded in round 3's convergence list, deferred again under the code-age ruledocs/design/2026-08-27-session-pr-aone-provider.md:161 — [review] design doc contradicts code on unrepaired-legacy-binding cost — deferred under the code-age rulepackages/cli/src/serve/routes/session-pr-backfill.ts:591 — [probe] repair commit-time re-check unwitnessed — deferred under the code-age rule
Convergence: round 4 posted 9 inline comment(s), 9 of them reported for the first time; the previous round posted 13 (0 new). Findings keep coming back to the same files: packages/cli/src/serve/server/session-pr-refresh.ts (findings in round 1; 4 more now); packages/cli/src/serve/server/aone-mrs.ts (findings in rounds 1, 2; 3 more now); packages/cli/src/serve/routes/session-pr-backfill.ts (findings in rounds 1, 2; 1 more now), and 1 more file(s). 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.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — stopped at the 5-round plan cap with round 5 still reporting (1 new finding); rounds beyond the cap were not run。
未探索到全部深度(达到工具调用预算):chunk 5:none — one check (actually executing the test file) was deliberately not run because the worktree has no installed dependencies; it was replaced by full manual …。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 9 条行内评论,其中 9 条是首次提出;上一轮发布了 13 条(其中 0 条首次提出)。发现反复回到同一批文件:packages/cli/src/serve/server/session-pr-refresh.ts(第 1 轮已出过发现,本轮又有 4 条);packages/cli/src/serve/server/aone-mrs.ts(第 1、2 轮已出过发现,本轮又有 3 条);packages/cli/src/serve/routes/session-pr-backfill.ts(第 1、2 轮已出过发现,本轮又有 1 条),另有 1 个文件。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
Address review round 4: - Make isAoneDetailUrlForRepo match ONLY the detailUrl spelling a1 produces (web host, https, port-less), mirroring the sidecar write path's exact canonicalSessionPrUrl equality. A binding in any other spelling (git host, http, an explicit port) is one the write path would also refuse — classifying it own/refreshable only spent a capped view slot on a state that could never land. Such entries now stay foreign-and-kept and are never viewed, closing the frozen-badge/wasted- slot family without loosening the cross-repo write guard. - Validate the a1 detailUrl against the sidecar's URL invariants before trusting it (parseAoneMrView): an oversized or control-character URL would make readSessionPrs reject the entire list, voiding all of a session's bindings. The view now degrades to "unresolved this run". - Report how many views the sweep actually started (aoneConsumed) and have the timer advance the rotating window by it instead of the fixed cap, so a budget-truncated sweep's tail is picked up next sweep rather than falling in the gap between windows. Only the Aone path writes an offset. - Prune a removed workspace's sweep offset each tick so the map cannot grow without bound in a long-running daemon. - Pin the rest: view-path version floor, below-floor re-probe on every read, git-host-spelled refresh exclusion, the detailUrl invariants, timer rotation and offset pruning.
|
🤖 Could not produce a passing fix for this feedback (round 1/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 未能为该反馈产生可通过验证的修复(第 1/100 轮)。此项现在需要人工处理;循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33144124481 🧠 Handled by Qwen Code · model/模型 |
|
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 |
|
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 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): chunk 9: could not execute the two new timer tests — the worktree has no node_modules anywhere in its tree, and a cold monorepo install + workspace build exceeds this pa….
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/server/aone-mrs.ts:332 — [probe] positive version-probe memoization unpinned — re-derived, deferred again (rounds 2, 4)packages/cli/src/serve/server/aone-mrs.ts:386 — [probe] list paging past page 1 unpinned — re-derived, deferred again (rounds 2, 3, 4)packages/cli/src/serve/server/aone-mrs.test.ts:265 — [review] a1ErrorMessage fallback branch (stderr preference, whitespace collapse, 512-char cap) untestedpackages/cli/src/serve/routes/session-pr-backfill.test.ts:948 — [review] repair commit-time re-check guard has no concurrency witnesspackages/cli/src/serve/routes/session-pr-backfill.ts:517 — [review] legacy repairs consume the shared view budget before planned numbers, no rotationpackages/cli/src/serve/routes/session-pr-backfill.ts:520 — [probe] failed legacy repairs increment no counter in the run summarypackages/cli/src/serve/routes/session-pr-backfill.ts:541 — [probe] view budget spent every run on numbers that can never take a slot (first reported round 3)packages/cli/src/serve/routes/session-pr-backfill.test.ts:619 — [probe] list-failure test never asserts the mr view call (degraded-mode invariant unpinned)packages/cli/src/serve/server/session-pr-refresh.test.ts:519 — [probe] sweepStart modulo reduction (refresh.ts:256) has zero witnesspackages/cli/src/serve/server/session-pr-refresh.test.ts:522 — [probe] rotation window indexes an unsorted, churn-reshuffled enumeration
Convergence: round 5 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 9 (9 new). Findings keep coming back to the same files: packages/cli/src/serve/server/session-pr-refresh.ts (findings in rounds 1, 4; 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.)
中文说明
未探索到全部深度(达到工具调用预算):chunk 9:could not execute the two new timer tests — the worktree has no node_modules anywhere in its tree, and a cold monorepo install + workspace build exceeds this pa…。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 10 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 5 轮发布了 3 条行内评论,其中 2 条是首次提出;上一轮发布了 9 条(其中 9 条首次提出)。发现反复回到同一批文件:packages/cli/src/serve/server/session-pr-refresh.ts(第 1、4 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
…10287) Address review round 5 of the Aone session-PR work: - R5-1 (Critical): the rotation/prune timer tests advanced the fake clock the instant the VIEW count landed, racing the tick's real-fs commit tail; under full-suite load the re-entrancy guard skipped the next tick and the tests went intermittently red. Wait on the sweep's observable completion signal (the bridge notification fired only after the last sidecar commit) before crossing each interval. - R1-4: thread a `now` clock seam through startSessionPrRefreshTimer into the sweep's aggregate-budget deadline, and add a timer-level test that truncates the window via the injected clock and asserts the next window begins at the consumed count, not the fixed cap. Pins the aoneConsumed advance against the cap mutant. - R4-4: add a timer test that closes the generation guard so a sweep throws non-draining, and asserts the next tick retries the same window instead of skipping it. Pins the offset-unmoved-on-throw invariant against the advance-in-catch / move-to-finally mutants. Verified: session-pr-refresh.test.ts green in isolation and under the full `npm test --workspace=packages/cli` load (the remaining suite failures are pre-existing, load-related flakes in unrelated files).
…nto feat/session-pr-aone-provider
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.8-max via Qwen Code /review (v0.22.2)
…10287) The R4-4 witness ('keeps the offset unmoved when a sweep throws, retrying the window') closed the guard and advanced straight to a view-count assertion, but the sweep's generation-guard assert sits AFTER its real-fs sidecar scan. Under realistic fs timing the test re-opened the guard before the sweep reached the assert, so the sweep never threw: the advance-in-catch mutant survived and the offset-unmoved-on-throw invariant was pinned by nothing. Wait on the assert itself — a spy on the test-owned closed guard — before concluding the tick threw. The advance-in-catch mutant now fails the window-retry assertion (21 !== 26); restored, the test is green in isolation and under the full packages/cli suite load.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #10287 (address-review)Feedback triageEvery item newer than the last evaluation was triaged:
No conflict ( What was verified and what changedThe three round-5 findings were already answered by commit
Changed file: Verification
Commit: 中文说明Autofix 轮次总结 — PR #10287(address-review)反馈分诊上次评估之后的所有新条目均已逐一分诊:
无冲突( 验证结果与本轮改动第 5 轮的三条发现此前已由提交
改动文件: 验证
提交: 🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。 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.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 6b": mutation-check the advance-in-catch witness (a concurrent process in this shared worktree inserted and later reverted exactly that mutant in session-pr-refresh.….
中文说明
未发现问题。LGTM!✅
未探索到全部深度(达到工具调用预算):"agent 6b":mutation-check the advance-in-catch witness (a concurrent process in this shared worktree inserted and later reverted exactly that mutant in session-pr-refresh.…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…eate Resolves the session-pr-backfill conflict with QwenLM#10287 (Aone as a first-class platform): the Aone provider now rides the branch's source model — /review commands and the worktree convention resolve through a capped, cached mr view (never a fabricated URL), legacy <origin>/pull/N bindings are repaired in place, and the transcript-branch mapping the branch removed as noise is not reintroduced for Aone (its mr list calls and tests go with it).
|
Released in v0.22.3. |
What this PR does
Session PR bindings record which pull requests a session produced. Manual binding already accepted any PR URL, but the two automatic paths — the binding backfill and the daemon's state-refresh sweep — only worked against GitHub. This PR makes Aone a first-class platform for both: each workspace's origin is inspected to pick the platform, and Aone workspaces get branch-to-MR mapping and state refresh through the
a1CLI. Every Aone binding URL now comes from the platform's own MR metadata (Aone links can't be reliably assembled from a remote), the same-PR identity guard that protects foreign same-numbered bindings fails closed on Aone, and bindings persisted by the old GitHub-only code in a fabricated shape are detected and repaired in place. The GitHub path is untouched, and the wire response only gains optional fields.Why it's needed
On an Aone workspace the old backfill bound sessions to fabricated
<origin>/pull/<N>pages that don't exist (Aone review pages live under/codereview/<id>on the web host), and the refresh sweep — which keeps the session-list badge state (open/merged/closed) fresh — only ever rangh, so an Aone binding's state froze at bind time forever. E2E against the pre-change build reproduced both: a convention-bound session gothttps://gitlab.alibaba-inc.com/<repo>/pull/<id>with no state, and a manually bound Aone MR stayed stateless across refresh sweeps. Aone-origin repos (the normal case for internal work) thus showed dead links and stale badges in the session list.Reviewer Test Plan
How to verify
The main path is covered by unit tests with an injected
a1backend (13 cases across the backfill and refresh suites: platform detection against real temp git repos, view-based convention resolution, branch mapping incl. reused-head highest-number-wins and the default-branch exclusion, opened-state listing, degraded mode when listing fails, view-failure handling, per-run/per-sweep view caps, foreign same-numbered binding protection, and legacy-URL repair), plus an E2E run against a real Aone repo with a reala1login. To verify manually with an Aone account:pr-<N>convention).POST /sessions/backfill-prs— expect the session bound to the realhttps://code.alibaba-inc.com/<group>/<project>/codereview/<global-id>URL with a state, response carryingplatform: "aone"andaoneAvailable: true; no/pull/URL anywhere.createdAtis preserved.<origin>/pull/<N>URL and re-run the backfill — the entry is rewritten to the real URL with state,createdAtintact (verified E2E; a failingmr viewleaves the entry untouched instead).Observed E2E (real
a1,jspt/agentic_coding): baseline bound the fabricatedgitlab.alibaba-inc.com/.../pull/26430560; after the change both sessions bindcode.alibaba-inc.com/jspt/agentic_coding/codereview/26430560withstate: "merged"; the sweep fills the state for a manual binding within one 1-minute window; a GitHub workspace backfill returns byte-identical bindings to baseline (plus the new additiveplatform: "github"field).Evidence (Before & After)
N/A (daemon-internal behavior; no TUI change — the Web Shell badge renders whatever the daemon stores, and the stored URL/state is what changed).
Tested on
Environment (optional)
Local daemon from the bundled build (
node dist/cli.js serve), reala10.2.51 authenticated viaa1 auth login, unit suite vianpx vitest runinpackages/cli.Risk & Scope
a1calls, bounded by design — 3 pages per state for branch mapping and a 25-call cap on MR views per backfill run / refresh sweep (excess retries on the next run/sweep), each call under a 20s timeout on the async runner. Workspaces with more than 25 unique non-merged bindings refresh deterministically from the head of the pending set (documented in the design doc as implausible in practice).a1(a reopened MR reappears as opened and self-heals); AGit-Flow repos whose MR source branch is a head SHA simply don't branch-map (convention binding unaffected).platform/aoneAvailablefields, and the sidecar schema is unchanged.Linked Issues
None.
中文说明
这个 PR 做了什么
会话 PR 绑定记录每个会话产出了哪些 PR。手动绑定本来就接受任意 PR URL,但两条自动路径——绑定回填(backfill)与 daemon 的状态定时刷新——此前只对 GitHub 生效。本 PR 让 Aone 在两条路径上都成为一等公民:按工作区 origin 判定平台,Aone 工作区通过
a1CLI 完成分支→MR 映射与状态刷新。每个 Aone 绑定的 URL 都来自平台自身的 MR 元数据(Aone 链接无法从 remote 可靠拼装);保护「外部仓库同号绑定」的同 PR 身份守卫在 Aone 上改为 fail-closed;旧版 GitHub-only 代码以虚构形状持久化的绑定会被识别并原地修复。GitHub 路径完全不变,接口响应只新增可选字段。为什么需要
在 Aone 工作区上,旧的回填会把会话绑定到虚构的
<origin>/pull/<N>页面——该页面并不存在(Aone 的评审页在 web 主机的/codereview/<id>下);而负责保持会话列表徽章状态(open/merged/closed)的定时刷新只跑gh,导致 Aone 绑定的状态在绑定那一刻起永久冻结。变更前的 E2E 复现了这两点:约定绑定的会话拿到https://gitlab.alibaba-inc.com/<repo>/pull/<id>且无 state;手动绑定的 Aone MR 跨多轮刷新仍无 state。于是内部常见的 Aone 仓库在会话列表里显示死链与过期徽章。审阅者测试计划
如何验证
主链路由注入
a1后端的单测覆盖(回填与刷新两个套件共 13 个用例:基于真实临时 git 仓库的平台检测、基于 view 的约定解析、分支映射含复用分支取最大号与默认分支排除、opened 状态列表、列表失败时的降级、view 失败处理、每次运行/每轮刷新的 view 上限、外部同号绑定保护、旧版虚构 URL 修复),另在真实 Aone 仓库 + 真实a1登录下跑了 E2E。有 Aone 账号可手动验证:pr-<N>约定的 worktree 会话)。POST /sessions/backfill-prs——期望会话被绑定到真实的https://code.alibaba-inc.com/<group>/<project>/codereview/<全局id>,带 state;响应含platform: "aone"与aoneAvailable: true;不出现任何/pull/URL。createdAt保持不变。<origin>/pull/<N>条目后重跑 backfill——该条目被重写为真实 URL 与 state,createdAt保留(已 E2E 验证;mr view失败时条目保持原样)。真实
a1(jspt/agentic_coding)上的 E2E 观察:基线绑定出虚构的gitlab.alibaba-inc.com/.../pull/26430560;变更后两个会话都绑定code.alibaba-inc.com/jspt/agentic_coding/codereview/26430560且state: "merged";手动绑定在一个 1 分钟刷新窗口内补上 state;GitHub 工作区的回填结果与基线逐字节一致(仅多出新增的platform: "github"字段)。前后对比证据
N/A(daemon 内部行为,无 TUI 变化——Web Shell 徽章渲染的是 daemon 存储的内容,本 PR 改变的是存储的 URL/state)。
测试环境
环境(可选)
本地 bundle 构建的 daemon(
node dist/cli.js serve)、a1 auth login认证的真实 a1 0.2.51,单测在packages/cli下用npx vitest run运行。风险与范围
a1调用,按设计受限——分支映射每状态最多 3 页,每次回填运行/每轮刷新的 MR view 上限 25 次(超出部分下次运行/刷新重试),每次调用 20s 超时且走异步 runner。若某工作区有超过 25 个未合并的唯一绑定,刷新从待处理集合头部确定性推进(设计文档中记录为实际不太可能发生)。a1无法列出已关闭的 MR(重新打开的 MR 会以 opened 重新出现、自愈);AGit-Flow 仓库的 MR 源分支是 head SHA 时不做分支映射(约定绑定不受影响)。platform/aoneAvailable,侧车 schema 不变。关联 Issue
无。