mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-31 02:06:21 +00:00
feat(review): back pr-context on Aone Code targets (#9621)
* feat(review): back pr-context on Aone Code targets 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. * fix(review): keep Aone ledger carriers out of the blocker re-check (#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. * fix(review): refuse pr_number spellings that do not round-trip (#9621) 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. * docs(review): align Aone docs with the landed no-ancestry anchor rule and comment-status skips D6 described the AGit-Flow anchor as inert until the incremental rule landed, but that rule (#9630) merged while this branch was in flight — anchors now delta-scope Aone re-reviews. SKILL.md's comment-status section and Step 6's report-existence guard now name the Aone skip the setup batch already carries, so no path sends an Aone run at the unbacked command or at a report that was never written. * docs(review): annotate #9616 as landed and define the report-less re-check rule The out-of-scope list still read self-PR detection as open work although #9629 shipped it into this branch's merge base — annotate it like the sibling #9618 entry. Step 6's report-existence guard pointed report-less runs at a re-derivation the skill never defines; replace it with the explicit rule: no per-thread status routing, no hand-derived substitute, rule from the code at the reviewed commit, cannot-tell over a guess. * fix(review): route the context head through aoneHeadSha and close the round-5 findings getReviewContext read sourceBranch raw while every other head read trims — a padded server value diverged the context file from the rest of the run (phantom-drift shape). getCurrentUser now honors the seam contract on the anomalous whoami shapes instead of leaking untagged throws and non-string accounts. Step 6's report-less rule no longer contradicts the comment-status failure contract: runs where the command ran and failed keep the "re-derive if needed" fallback. The Aone paragraph names comment-body among the backed reads, and witness tests pin the identity gate's carriers key and the head normalization. * fix(review): shape-check the Aone comment listing in getReviewContext a1 can answer repo mr comment list with an exit-0 a1.error/v1 error object (backend auth failure or client timeout — measured by cleanup's a1CommentList on the identical payload). Without a guard the object survives the ?? [] coalesce and .filter throws an untagged TypeError, losing the envelope's actionable message at exactly the recoverable moment. Guard as the provider family already does and surface the cause; witness tests pin both envelope shapes (mutant-checked). * test(review): pin getCommentBody's body-field fallback (mutant-checked) * fix(review): union resolved comments into the Aone context bundle The default comment list excludes resolved comments (measured by the cleanup audit) while GitHub's REST fetches include them, so a resolved blocker/marker root never reached the re-check walk or the fail-closed identity gate. Union the default and --resolved listings as the audit does, dedupe by id, fail closed on either listing's error envelope, and disclose the residual that resolved replies stay invisible; witness tests mutant-checked. * fix(review): serve resolved comments and guard the envelope in getCommentBody 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. * ci: correct qwen-autofix.yml size baseline to its actual post-migration 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. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
This commit is contained in:
Shaojin Wen
GitHub
parent
6b729f91d6
commit
ec8a8a1a97
13 changed files with 1719 additions and 137 deletions
|
|
@ -493,6 +493,23 @@ Enterprise paragraph.
|
|||
dedup backing for Aone, cleanup audit, the ai_comment marking flag
|
||||
(a1-side), the render-adjudication carve-out.
|
||||
|
||||
- **Phase 3b — Aone `pr-context` backing (this change).** The reader gains
|
||||
`getReviewContext` + `getCurrentUser` (D1's `getContext` + `self`,
|
||||
synchronous). `pr-context` routes through the platform reader; the
|
||||
normalized bundle keeps ALL rendering and security logic platform-neutral.
|
||||
GitHub's implementation EXTRACTS pr-context's existing gh calls
|
||||
unchanged — the existing suite passing unmodified is the no-regression
|
||||
evidence. On Aone: metadata from `mr view` (stats degrade), one flat
|
||||
comment list split by `path`, no verdicts, ledger carriers = the
|
||||
thread-level comments (the posted summaries); refetch commands bake
|
||||
`--pr` (Aone addresses every comment body per-MR) and bake only an
|
||||
explicit `--host` (never the ambient GH_HOST). The forced
|
||||
context-unavailable cap leaves submit (the reads are backed now), so an
|
||||
Aone run that read its context can APPROVE and the wired
|
||||
`a1 repo mr approve` fires. Agent 0 becomes runnable on Aone (its gate
|
||||
is pr-context success; its welded `issue-context` command is already
|
||||
backed). Design: `2026-08-21-review-aone-pr-context.md`. Still open: the
|
||||
Phase-3 open items above, unchanged.
|
||||
- **Phase 4 — semantic gaps.** Incremental-cache ancestry fallback, build-test
|
||||
repo-config escape hatch, publish-assets gating polish, generic-GitLab
|
||||
(glab) evaluation.
|
||||
|
|
|
|||
280
docs/design/2026-08-21-review-aone-pr-context.md
Normal file
280
docs/design/2026-08-21-review-aone-pr-context.md
Normal file
|
|
@ -0,0 +1,280 @@
|
|||
# Phase 3b: Aone Code `pr-context` backing for /review
|
||||
|
||||
> Status: draft. Scope: back `qwen review pr-context` on Aone Code through the
|
||||
> platform reader, lifting the forced context-unavailable cap on Aone runs.
|
||||
> Parent: `2026-08-13-review-platform-provider-abstraction.md` (Phase 3,
|
||||
> "still open: context reads"). Continues `2026-08-15-review-aone-provider.md`.
|
||||
|
||||
## Context
|
||||
|
||||
The Aone read path (Phase 2, #9226) and the Aone write path (Phase 3 `submit`
|
||||
slice, #9491) are landed. But `pr-context` — the subcommand that fetches a
|
||||
target's metadata and existing discussion into the context file every agent
|
||||
reads — is still GitHub-direct. On an Aone target it is SKIPPED, and the skip
|
||||
cascade defines the current Aone experience:
|
||||
|
||||
- every Aone run is context-unavailable; `submit` FORCES the cap for Aone
|
||||
writes, so the verdict can never rise past COMMENT;
|
||||
- the wired `a1 repo mr approve` never fires (the cap precedes it);
|
||||
- Agent 0 (issue fidelity) is skipped — it is gated on `pr-context` success,
|
||||
even though `issue-context` (its evidence command) is already Aone-backed;
|
||||
- the machine ledger is never recovered from the MR, so every round re-opens
|
||||
as round 1 even though the posted summary comment carries the marker.
|
||||
|
||||
Backing `pr-context` is the single change that lifts all four: the cap is a
|
||||
consequence of the skip, Agent 0's gate is `pr-context` success, and the
|
||||
ledger lives in the posted comments this subcommand would now read.
|
||||
|
||||
Out of scope (tracked): comment-status/presubmit backing and cross-round
|
||||
dedup (#9613), AI-comment marking (#9614), removed-line anchors (#9615),
|
||||
self-PR detection (#9616 — landed via #9629 and merged into this branch
|
||||
while it was in flight), cleanup audit (#9617), incremental cache under
|
||||
AGit-Flow (#9618 — landed via #9630 and merged into this branch while it was
|
||||
in flight; see D6), test-plan routing / composeUrl / a1 version floor (#9619).
|
||||
|
||||
## Verified platform facts
|
||||
|
||||
The Phase-2 facts table (probed 2026-08-13 against maxcompute/odps_src with
|
||||
a1 v0.1.90) already covers everything this phase reads, plus the a1 command
|
||||
surface re-checked 2026-08-21 via help (no auth needed):
|
||||
|
||||
| Need | a1 surface | Notes |
|
||||
| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- |
|
||||
| MR metadata | `a1 repo mr view <id> -f json` → `title, description, state, sourceBranch (head SHA under AGit-Flow), targetBranch, author, detailUrl` | no additions/deletions stats — the context header degrades |
|
||||
| All comments | `a1 repo mr comment list --mr <id> -f json [--sort asc]` → `id, note, author, closed, outdated, path, line, side, parentNoteId, isAiComment, isDraft` | one flat collection; `path` present ⇔ inline; NO pagination flags (full list returned); default sort `desc` |
|
||||
| Reviews / verdicts | none — no review object exists on Aone | approvals surface only through `mr status` checks |
|
||||
| Current user | `a1 auth whoami -f json` → `account` | |
|
||||
|
||||
Probe-pending shapes RESOLVED 2026-08-21 against live MRs once a1 auth was
|
||||
restored:
|
||||
|
||||
- `author` is an object `{id, name, username}` — `username` is the identity
|
||||
space `a1 auth whoami`'s `account` answers in (verified both on one MR).
|
||||
The tolerant reader tries `account` first, then `username`.
|
||||
- `createdAt` is present (ISO-8601 with offset); `closed` is a NUMBER,
|
||||
`outdated`/`isAiComment`/`isAiSummary`/`isDraft` are booleans — the draft
|
||||
skip is live, not hypothetical.
|
||||
- The comment envelope also carries `updatedAt`, `adopted`, `labels` —
|
||||
unread.
|
||||
|
||||
One NEW gap surfaced by the same E2E (filed as #9620, pre-existing, out of
|
||||
this phase's scope): `mr view` carries NO head-SHA field, and
|
||||
`sourceBranch` is a branch NAME on branch-based (non-AGit-Flow) MRs — the
|
||||
provider's sourceBranch-as-SHA assumption (facts table, submit's drift gate,
|
||||
`meta`'s headSha) only holds under AGit-Flow.
|
||||
|
||||
## Design decisions
|
||||
|
||||
### D1 — One new reader operation, `getReviewContext`, plus `getCurrentUser`
|
||||
|
||||
`pr-context` is the single consumer, so the reader gains exactly two members:
|
||||
|
||||
- `getReviewContext(prNumber, ownerRepo): ReviewContext` — the normalized
|
||||
bundle: metadata, the flat comment list split into inline/thread channels,
|
||||
and the platform's review-level verdicts.
|
||||
- `getCurrentUser(): string` — the authenticated account ('' on the
|
||||
empty-output shape; throws on lookup failure).
|
||||
|
||||
The end-state interface in the parent doc named this `getContext(req)` —
|
||||
"description, comments, verdicts, self"; `self` stays a separate operation
|
||||
because pr-context's identity policy is deliberately NOT "always fetch": the
|
||||
lookup runs only when comments exist, and its failure semantics (fail closed
|
||||
iff a posted root carries a critical marker) are pr-context's security
|
||||
logic, not the platform's. Keeping that logic in one place, platform-neutral,
|
||||
is the point.
|
||||
|
||||
```ts
|
||||
interface ReviewContextComment {
|
||||
id: number;
|
||||
author: string; // login/account, '' when absent
|
||||
body: string;
|
||||
createdAt: string; // ISO; '' when the platform gives none
|
||||
path?: string; // present ⇔ inline comment
|
||||
line?: number;
|
||||
parentId?: number; // GitHub in_reply_to_id / Aone parentNoteId
|
||||
}
|
||||
|
||||
interface ReviewContextVerdict {
|
||||
id: number;
|
||||
author: string;
|
||||
body: string;
|
||||
state: string; // APPROVED | CHANGES_REQUESTED | COMMENTED | …
|
||||
submittedAt: string;
|
||||
commitId?: string; // GitHub: the head the review was submitted against
|
||||
}
|
||||
|
||||
interface ReviewContext {
|
||||
title: string;
|
||||
body: string;
|
||||
authorLogin: string;
|
||||
state: string;
|
||||
baseRefName: string;
|
||||
headRefName: string; // branch name (GitHub); Aone: sourceBranch — a bare SHA under AGit-Flow
|
||||
headRefOid: string;
|
||||
additions?: number; // absent where the platform reports no stats
|
||||
deletions?: number;
|
||||
changedFiles?: number;
|
||||
comments: ReviewContextComment[];
|
||||
verdicts: ReviewContextVerdict[];
|
||||
/** Where this platform's machine-ledger markers live, shaped as verdicts
|
||||
* ready for `recoverLedger`: GitHub — the review bodies; Aone — the
|
||||
* thread-level comments (the posted summaries). */
|
||||
ledgerCarriers: ReviewContextVerdict[];
|
||||
}
|
||||
```
|
||||
|
||||
`github.ts`'s implementation EXTRACTS pr-context's existing calls (one
|
||||
`gh pr view` + the three paginated `ghApiAll` fetches; `ledgerCarriers` =
|
||||
`verdicts`). The transport seam stays `lib/gh.js`, so pr-context's existing
|
||||
test suite — which mocks that module — pins the extraction unchanged. That
|
||||
suite passing unmodified is the no-regression evidence.
|
||||
|
||||
### D2 — Aone channel mapping
|
||||
|
||||
One flat comment collection serves three GitHub channels:
|
||||
|
||||
- `path` present → inline comment; `parentNoteId` → `in_reply_to_id`, so
|
||||
`classifyInlineThreads`/`findRootId` walk Aone threads unchanged.
|
||||
- `path` absent → issue-level ("general MR thread") comment — blocker
|
||||
promotion and the "Already discussed" channel, exactly the treatment
|
||||
GitHub's issue comments get.
|
||||
- ledger carriers = the path-ABSENT comments, mapped into the verdict shape
|
||||
(`state: 'COMMENTED'`, no `commitId`): the qwen summary lands there, and
|
||||
`recoverLedger` walks carriers — own/foreign split, round-first selection,
|
||||
headroom, the merge-over-own union — with zero Aone-specific code. Aone
|
||||
comments carry no `commit_id`, so a recovered Aone ledger has no age
|
||||
reference — the convergence posture already skips its age rule on that
|
||||
shape ("skip the age rule, not the review").
|
||||
- `isDraft` comments are skipped (the PENDING-review analogue: a draft is
|
||||
not a posted round and not settled discussion).
|
||||
- Resolved comments are INCLUDED: the default `comment list` excludes them
|
||||
(measured: the MR's `comments` minus `closedComments`), while GitHub's
|
||||
REST fetches include resolved-thread comments — so `getReviewContext`
|
||||
unions the default and `--resolved` listings, deduped by id, the same
|
||||
union `cleanup`'s audit applies. The `closed`/`outdated` FLAGS are still
|
||||
ignored this phase (their consumer is comment-status, #9613); the union
|
||||
is about which comments ARRIVE, not how their state is rendered.
|
||||
Disclosed residual: resolved REPLIES stay invisible — the `--resolved`
|
||||
listing returns resolved ROOT inline comments only, so a resolved thread
|
||||
renders its root without its reply chain (the re-check walk is unaffected
|
||||
— a reply alone never retires a blocker).
|
||||
- `verdicts` is empty: Aone has no review object. The "Review summaries"
|
||||
section therefore renders nothing on Aone; human overall comments are
|
||||
thread comments and render in the thread channels like GitHub issue
|
||||
comments. Approvals are visible only through `mr status` and join with the
|
||||
dedup/presubmit phase.
|
||||
|
||||
### D3 — Metadata mapping
|
||||
|
||||
From `mr view`: `title`, `description` → body, `author` → authorLogin,
|
||||
`state` passthrough, `targetBranch` → baseRefName, `sourceBranch` →
|
||||
headRefOid (it IS the head SHA under AGit-Flow). `headRefName` =
|
||||
`sourceBranch` as well: under AGit-Flow that string is the head SHA, and
|
||||
rendering `master ← <sha>` in the context header is truthful and
|
||||
informative; a non-AGit-Flow MR's real branch name renders the same way
|
||||
GitHub's does. Diff stats have no Aone source: the header line degrades to
|
||||
"not reported by the platform" instead of printing zeros (zeros would assert
|
||||
an empty diff). pr-context does not compute them locally — the worktree
|
||||
belongs to fetch-pr, and duplicating its merge-base arithmetic here would be
|
||||
a second copy of logic that has a home.
|
||||
|
||||
### D4 — Refetch commands bake `--pr` on Aone; only an explicit `--host` bakes
|
||||
|
||||
pr-context's truncation notes emit `comment-body` commands. On GitHub,
|
||||
inline and issue comment ids are global, so only `--kind review` carries
|
||||
`--pr`; on Aone every comment body is addressed per-MR (comment-body already
|
||||
refuses a pr-less Aone call). The emitted command builder learns the
|
||||
platform: on Aone, every emitted refetch carries `--pr <id>`. A refetch a
|
||||
reader cannot run is a truncation nobody can complete, which the fail-closed
|
||||
"partial read is `cannot tell`" rule then turns into a stalled re-check.
|
||||
|
||||
The host half of the contract: on Aone only an EXPLICIT `--host` flag bakes
|
||||
into the emitted refetches. An ambient `GH_HOST` never does — it is a
|
||||
different platform's host, and baking it would silently retarget every
|
||||
refetch at a GitHub host, re-opening the exact cross-platform leak the
|
||||
`--pr` rule closes. A flagless Aone run's refetches rely on the cwd clone's
|
||||
origin — the same detection the run itself used. (GitHub keeps its existing
|
||||
policy: the explicit `--host` else an operator-exported `GH_HOST` bakes.)
|
||||
|
||||
### D5 — The forced context-unavailable cap leaves the Aone write path
|
||||
|
||||
`submit` forces `contextUnavailable: true` for Aone writes because "this
|
||||
phase has no Aone backing for pr-context" — the premise this change removes.
|
||||
The force goes away; the Aone path takes the state's claim exactly as GitHub
|
||||
does (handed through raw, a malformed value refused by compose-review). The
|
||||
forgery class the force closed — a forged/omitted field composing an APPROVE
|
||||
— reopens to exactly the level GitHub runs at, where the reads are backed
|
||||
and the claim stands. comment-status/presubmit remain unbacked, but they
|
||||
feed the `presubmit` downgrade fields, never `contextUnavailable` (whose
|
||||
meaning is "pr-context failed or was skipped", per Step 1). Consequence: an
|
||||
Aone run that read its context can now compose APPROVE, and the wired
|
||||
`a1 repo mr approve` fires for the first time.
|
||||
|
||||
### D6 — Ledger anchors under AGit-Flow: carried, live via the no-ancestry rule
|
||||
|
||||
A recovered own-ledger's `sha` rides into the side file and the section's
|
||||
anchor ruling as on GitHub. Under AGit-Flow the anchored head is amended
|
||||
(orphaned) on every update, so the ancestry test GitHub's incremental path
|
||||
relies on would fail for EVERY update; `fetch-pr --since` therefore
|
||||
resolves Aone anchors with the no-ancestry rule (the parent doc's D7,
|
||||
shipped by #9630 while this branch was in flight): the anchor-behind-head
|
||||
test and the merge-base clamp are both skipped, and a recovered anchor
|
||||
delta-scopes the round to the files the update touched — rebase drift
|
||||
staying within them keeps the scope, anything wider falls back to
|
||||
full-range, and no drift byte enters the published scope. The anchor is
|
||||
LIVE on Aone, not inert: the draft's "inert until #9618" text predates that
|
||||
landing and was wrong from the merge on.
|
||||
|
||||
### D7 — Skill and doc surface
|
||||
|
||||
SKILL.md's Aone paragraph: every Aone run is NO LONGER context-unavailable;
|
||||
`pr-context` runs like GitHub (same failure handling — warn, continue,
|
||||
context-unavailable state); Agent 0 runs (its welded `issue-context` command
|
||||
is Aone-backed); `plan-diff` gets `--pr`/`--repo` when the fetch succeeds.
|
||||
The skip list shrinks to what is still unbacked: `comment-status`,
|
||||
`presubmit`, `test-plan`, the Step 9 bypass audit, and `publish-assets`.
|
||||
The repeat-round caveats shrink to the two that remain true (no dedup
|
||||
backing; no self-PR detection). The "approve does not fire this phase" text
|
||||
goes. `docs/users/features/code-review.md`'s Aone paragraph and the parent
|
||||
doc's phase tracker update with it. (Post-write merges shrink the list
|
||||
again: #9629 lands presubmit's self-PR/drift backing and #9633 the Aone
|
||||
bypass audit — the merged SKILL.md and user doc carry the live state; this
|
||||
paragraph records the phase-3b scope as written.)
|
||||
|
||||
## Files affected
|
||||
|
||||
- `lib/platform/types.ts` — `ReviewContext` types; the two reader members.
|
||||
- `lib/platform/github.ts` — implementation (extraction of pr-context's
|
||||
current calls; `currentUser` passthrough).
|
||||
- `lib/platform/aone.ts` — implementation over `mr view` + `comment list`;
|
||||
whoami mapping.
|
||||
- `pr-context.ts` — routes through `getPlatformReader({host})`; keeps ALL
|
||||
rendering and security logic (blocker promotion, ledger recovery,
|
||||
fail-closed identity, truncation discipline) platform-neutral over the
|
||||
normalized bundle; PrMetadata's stats become optional; refetch commands
|
||||
gain `--pr` on Aone.
|
||||
- `submit.ts` — the forced cap removed (D5), its comment rewritten.
|
||||
- `SKILL.md`, `docs/users/features/code-review.md`,
|
||||
`docs/design/2026-08-13-review-platform-provider-abstraction.md`
|
||||
(phase tracker).
|
||||
|
||||
Tests: pr-context's suite passes unmodified (transport seam unchanged);
|
||||
new tests in `lib/platform/aone.test.ts` for the mapping (channel split,
|
||||
draft skip, carrier shaping, author fallbacks, stats absence), in
|
||||
`pr-context.test.ts` for the Aone routing (emitted `--pr` refetches,
|
||||
degraded diff line, ledger recovery from comments), and the `submit` suites'
|
||||
forced-cap pins flip to parity pins.
|
||||
|
||||
## Open questions
|
||||
|
||||
1. ~~The probe-pending shapes above (author field, createdAt, isDraft
|
||||
visibility).~~ RESOLVED — see the facts table addendum; the mapping tests
|
||||
are pinned to the live shapes.
|
||||
2. Comment volume: a long-lived odps_src CR's comment list arrives in one
|
||||
un-paginated JSON payload; the 64 MiB aone-client bound covers it, but a
|
||||
monster thread's context file crosses the read_file threshold — the
|
||||
existing size warning + paging guidance already handles that shape.
|
||||
3. Branch-based (non-AGit-Flow) MRs carry a branch NAME in `sourceBranch`
|
||||
and `mr view` exposes no head-SHA field — the provider's
|
||||
sourceBranch-as-SHA assumption breaks on them (submit's drift gate then
|
||||
refuses every post). Pre-existing, filed as #9620, out of this phase.
|
||||
|
|
@ -382,7 +382,7 @@ The deterministic halves of the pipeline — argument parsing (`qwen review pars
|
|||
|
||||
**GitHub Enterprise:** reviewing a PR URL on a non-`github.com` host routes every GitHub call at that host — the review subcommands (`match-remote`, `meta`, `fetch-pr`, `pr-context`, `comment-status`, `issue-context`, `fetch-diff`, `comment-body`, `plan-diff`, `test-plan`, `presubmit`, `compose-review`, `submit`, `publish-assets`) accept `--host` and set it in code, so a forgotten host cannot silently retarget the review at `github.com`.
|
||||
|
||||
**Aone Code:** for a clone whose origin is on `gitlab.alibaba-inc.com`, run `/review` from inside that clone — the platform is detected from the remote and the subcommands work, backed by the `a1` CLI (at least 0.1.90 — an older install is refused at authentication time with an upgrade message) — the target number is the global MR id. `fetch-pr` fetches `refs/merge-requests/<id>/head` and builds the worktree + diff, so the agent review of the worktree is unchanged, and `test-plan` works too — it reads the MR description through the same reader. Every Aone run is context-unavailable and several flows are skipped (rather than hitting github.com's same-named repo): `pr-context`/`comment-status` have no Aone backing (the context-unavailable cap keeps an Approve verdict at Comment), `presubmit` is backed only for self-PR detection (the `a1 auth whoami` account vs the MR author) and head drift — its CI and existing-comment checks come back neutral — Agent 0 is skipped, and the `publish-assets` write is skipped. `--comment` **posts** the review through the `a1` CLI: one comment per inline finding, then the summary comment. Aone has no native request-changes state — on that verdict the summary comment carries a blocking header, and any inline Criticals that were actually posted block the merge through the discussion gate while their discussions stay unresolved (when no inline Critical posted, the header is advisory and nothing mechanically blocks the merge). The posted comments carry no AI-comment flag — `a1` cannot set one — so a repo's dedicated `ai_comment` merge gate does not track them. The native `a1 repo mr approve` is wired for an Approve verdict but does not fire this phase: the context-unavailable cap keeps an Approve verdict at Comment (a Request-changes verdict still posts its blocking summary). Incremental re-review follows the AGit-Flow update model: an update AMENDS the single CR commit in place, orphaning the head the previous round reviewed — so the cached anchor is ruled WITHOUT ancestry (the anchor-behind-head test would fail for every update), and the re-review scopes the PR's own diff to the files the update touched instead of falling back to a full review; an update that also rebased onto newer master keeps that scope only while the rebase's drift stays within the CR's files — drift touching any other file falls back to the full review, and no drift byte enters the published scope either way. One caveat for repeat rounds: there is no dedup backing yet, so a second `--comment` round re-posts every still-valid finding as a new comment (self-PR detection IS backed: reviewing your own MR gets the same verdict downgrade as on GitHub). See `docs/design/2026-08-15-review-aone-provider.md`.
|
||||
**Aone Code:** for a clone whose origin is on `gitlab.alibaba-inc.com`, run `/review` from inside that clone — the platform is detected from the remote and the subcommands work, backed by the `a1` CLI (at least 0.1.90 — an older install is refused at authentication time with an upgrade message) — the target number is the global MR id. `fetch-pr` fetches `refs/merge-requests/<id>/head` and builds the worktree + diff, so the agent review of the worktree is unchanged, and `test-plan` works too — it reads the MR description through the same reader. `pr-context` is backed too: it reads the MR's metadata, discussion threads, and previously posted qwen summaries (the machine ledger recovers from them), so an Aone run sees the MR's existing discussion exactly as a GitHub run sees a PR's. Several flows are still skipped or reduced (rather than hitting github.com's same-named repo): `comment-status` has no Aone backing, `presubmit` is backed only for self-PR detection (the `a1 auth whoami` account vs the MR author) and head drift — its CI and existing-comment checks come back neutral — and the `publish-assets` write is skipped. `--comment` **posts** the review through the `a1` CLI: one comment per inline finding, then the summary comment. Aone has no native request-changes state — on that verdict the summary comment carries a blocking header, and any inline Criticals that were actually posted block the merge through the discussion gate while their discussions stay unresolved (when no inline Critical posted, the header is advisory and nothing mechanically blocks the merge). The posted comments carry no AI-comment flag — `a1` cannot set one — so a repo's dedicated `ai_comment` merge gate does not track them. The native `a1 repo mr approve` fires for an Approve verdict when the run read the MR's context (the same gate as GitHub; a context-unavailable run stays capped at Comment). Incremental re-review follows the AGit-Flow update model: an update AMENDS the single CR commit in place, orphaning the head the previous round reviewed — so the cached anchor is ruled WITHOUT ancestry (the anchor-behind-head test would fail for every update), and the re-review scopes the PR's own diff to the files the update touched instead of falling back to a full review; an update that also rebased onto newer master keeps that scope only while the rebase's drift stays within the CR's files — drift touching any other file falls back to the full review, and no drift byte enters the published scope either way. One caveat for repeat rounds: there is no dedup backing yet, so a second `--comment` round re-posts every still-valid finding as a new comment (self-PR detection IS backed: reviewing your own MR gets the same verdict downgrade as on GitHub). See `docs/design/2026-08-15-review-aone-provider.md`.
|
||||
|
||||
Every run ends with one machine-readable line (`Review complete: <target> — <disposition>`), so scripts and CI wrappers can detect completion and outcome with a single `^Review complete: ` match.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue