fix(review): make the incremental cache work for Aone AGit-Flow CRs

docs/design/2026-08-13-review-platform-provider-abstraction.md

@@ -387,6 +387,31 @@ Enterprise paragraph.
 - **Phase 4 — semantic gaps.** Incremental-cache ancestry fallback, build-test
   repo-config escape hatch, publish-assets gating polish, generic-GitLab
   (glab) evaluation.
+  - **Landed (2026-08-21): the incremental-cache ancestry fallback (D7,
+    #9618).** `resolveIncrementalAnchor` gained a `noAncestry` mode that
+    `fetch-pr` selects when the platform is Aone: an AGit-Flow update
+    AMENDS the single CR commit in place, orphaning the cached head, so
+    the anchor-behind-head test failed for EVERY update and an
+    amend-and-re-review never scoped. Both ancestry tests — the
+    anchor-behind-head test and the behind-merge-base clamp — are
+    skipped (the clamp only ever fired when the update ALSO rebased onto
+    newer master, moving the merge base past the cached head; a pure
+    amend passed it); after the fetch both heads are local, so
+    `anchor..head` IS the update's delta, and the narrowing step
+    assembles the published scope from the CR's own diff exactly as it
+    does for an ancestrally valid GitHub anchor (an amended-and-rebased
+    update's delta carries the rebase drift, but the join reads it only
+    for which files changed — no drift byte reaches the published scope
+    — and drift touching a file outside the CR's diff falls back to the
+    full range there).
+    The existence checks and the `base-untrusted` refusal stay — they
+    guard presence and the base-derived capture, not the lineage. The
+    head-drift checks Aone has were confirmed to compare the live
+    `sourceBranch` SHA against the reviewed SHA the same way D7 names —
+    submit's pre-write gate and mid-batch re-read, and fetch-pr's resume
+    probe; none consults a platform compare API or an ancestry test.
+    GitHub keeps the tests: there an ancestor-less anchor is a
+    force-push, and the tests are the detection.
 
 ## Testing strategy
 

docs/users/features/code-review.md

@@ -363,7 +363,7 @@ If you switch models (via `/model`) and re-review the same PR, `/review` detects
 
 The model match also gates incremental scoping, not just the skip: "clean up to the cached commit" is the previous model's verdict, so when new commits have landed since the cached review, a model mismatch never scopes to `lastCommitSha..HEAD` — the range is the full diff, noting "Previous round was reviewed by qwen3-coder. Running full review with gpt-4o." — unless an anchor certified by the model now running is recovered from the last posted review (below), which scopes the range instead. The previous round's findings still carry over to be re-ruled; only the anchor does not. The same gate binds the anchor recovered from the last posted review's machine-ledger marker when the cache is absent or its anchor is unusable (CI, another clone): it scopes the incremental range only if the model now running certified it — a marker certified by a different model, or carrying no model (a review posted with `review.attribution` off, or one from before the field), falls back to the full diff.
 
-Cache is stored in `.qwen/review-cache/` and tracks both the commit SHA and model ID. Make sure this directory is in your `.gitignore` (a broader rule like `.qwen/*` also works). If the cached commit was rebased away, it falls back to a full review. Only high-effort reviews consult or write the cache — a `--effort low|medium` quick pass never counts as "already reviewed".
+Cache is stored in `.qwen/review-cache/` and tracks both the commit SHA and model ID. Make sure this directory is in your `.gitignore` (a broader rule like `.qwen/*` also works). On GitHub, if the cached commit was rebased or force-pushed away, it falls back to a full review; Aone rules the cached anchor differently — see its paragraph below. Only high-effort reviews consult or write the cache — a `--effort low|medium` quick pass never counts as "already reviewed".
 
 ## Review Reports
 
@@ -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 — 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. Every Aone run is context-unavailable and several flows are skipped (rather than hitting github.com's same-named repo): `pr-context`/`comment-status`/`presubmit` have no Aone backing (verdict caps at `COMMENT`), `test-plan` is unbacked, 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 every Aone verdict at Comment. Two caveats for repeat rounds: there is no dedup backing yet, so a second `--comment` round re-posts every still-valid finding as a new comment, and self-PR detection has no Aone backing. 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 — 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. Every Aone run is context-unavailable and several flows are skipped (rather than hitting github.com's same-named repo): `pr-context`/`comment-status`/`presubmit` have no Aone backing (verdict caps at `COMMENT`), `test-plan` is unbacked, 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 every Aone verdict 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. Two caveats for repeat rounds: there is no dedup backing yet, so a second `--comment` round re-posts every still-valid finding as a new comment, and self-PR detection has no Aone backing. 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.
 

packages/cli/src/commands/review/fetch-pr.test.ts

@@ -1261,6 +1261,24 @@ describe('fetch-pr report assembly', () => {
     );
   }
 
+  /**
+   * The ORPHANED-anchor shape, the complement of `anchorIsValid`: the
+   * anchor EXISTS and resolves to a commit, but every ancestry question
+   * answers null (exit 1) — a rebased-away GitHub anchor, or an Aone head
+   * orphaned by the AGit-Flow amend. One shape, so the tests that refuse
+   * it on GitHub and the tests that scope it on Aone cannot drift apart.
+   */
+  function serveOrphanShape(): void {
+    producerMocks.gitOpt.mockImplementation((...args: string[]) =>
+      args[0] === 'cat-file' ? '' : args[0] === 'rev-parse' ? ANCHOR : null,
+    );
+    producerMocks.resolveMergeBase.mockReturnValue({
+      sha: BASE,
+      baseFetchFailed: false,
+    });
+    servesBothRanges();
+  }
+
   it('pulls a still-clean importer of a changed file back into the scope', async () => {
     // The narrowing is sound in one direction only. `b.ts` has not changed
     // since the anchor, so the delta capture cannot show it and the narrowed
@@ -2181,15 +2199,7 @@ describe('fetch-pr report assembly', () => {
     // The `effective` clause in the partition guard: without it a round
     // whose anchor was refused for a deterministic reason gets relabelled
     // `partition-failed`, which invites re-running a dead anchor.
-    producerMocks.gitOpt.mockImplementation(
-      (...args: string[]) =>
-        args[0] === 'cat-file' ? '' : args[0] === 'rev-parse' ? ANCHOR : null, // not an ancestor
-    );
-    producerMocks.resolveMergeBase.mockReturnValue({
-      sha: BASE,
-      baseFetchFailed: false,
-    });
-    servesBothRanges();
+    serveOrphanShape(); // not an ancestor
     producerMocks.buildDiffPlan.mockImplementation((text: unknown) => {
       if (typeof text === 'string' && text.trim() !== '') {
         throw new Error('chunks do not tile the diff');
@@ -2274,14 +2284,8 @@ describe('fetch-pr report assembly', () => {
   });
 
   it('refuses a rebased-away anchor end to end, on a full-range plan', async () => {
-    producerMocks.gitOpt.mockImplementation(
-      (...args: string[]) =>
-        args[0] === 'cat-file' ? '' : args[0] === 'rev-parse' ? ANCHOR : null, // every merge-base probe fails → not an ancestor
-    );
-    producerMocks.resolveMergeBase.mockReturnValue({
-      sha: BASE,
-      baseFetchFailed: false,
-    });
+    // Every merge-base probe fails → not an ancestor.
+    serveOrphanShape();
     producerMocks.gitRaw.mockImplementation((...args: string[]) =>
       args.includes(`${BASE}..f00df00df00d`)
         ? Buffer.from(DELTA_DIFF)
@@ -2454,15 +2458,8 @@ describe('fetch-pr report assembly', () => {
     // reason announced "no diff could be captured" moments after the capture
     // succeeded and the partitioner warned, sending whoever diagnoses the
     // round at git and the network instead of at the partitioner.
-    producerMocks.gitOpt.mockImplementation((...args: string[]) =>
-      // `merge-base` answers null → exit 1 → the predicate's NO.
-      args[0] === 'cat-file' ? '' : args[0] === 'rev-parse' ? ANCHOR : null,
-    );
-    producerMocks.resolveMergeBase.mockReturnValue({
-      sha: BASE,
-      baseFetchFailed: false,
-    });
-    servesBothRanges();
+    // `merge-base` answers null → exit 1 → the predicate's NO.
+    serveOrphanShape();
     producerMocks.buildDiffPlan.mockImplementation((text: unknown) => {
       if (typeof text === 'string' && text.trim() !== '') {
         throw new Error('chunks do not tile the diff');
@@ -2697,6 +2694,91 @@ describe('fetch-pr report assembly', () => {
       expect(report.effort).toBeUndefined();
     });
   });
+
+  describe('the Aone incremental rule (AGit-Flow, D7, #9618)', () => {
+    // An AGit-Flow update AMENDS the single CR commit in place: the new
+    // head has the cached head's parent, never the cached head itself, so
+    // the head test answers "no" for the cached anchor on EVERY update —
+    // and the clamp too once the update also rebased onto newer master.
+    // The Aone rule must not ask either — after the fetch both heads are
+    // local, and `anchor..head` is the update's delta. Driven through the
+    // real handler with the Aone reader: an explicit Aone `--host` selects
+    // it, a mocked `a1` serves auth + MR view, and the git probes answer
+    // the orphan shape (existence yes, ancestry exit 1).
+
+    function serveAone(): void {
+      producerMocks.execFileSync.mockImplementation(
+        (cmd: string, args: string[]) => {
+          if (cmd !== 'a1') return '';
+          if (args[0] === 'repo' && args[1] === 'mr' && args[2] === 'view') {
+            return JSON.stringify({
+              mergeRequest: {
+                sourceBranch: 'f00df00df00d',
+                targetBranch: 'main',
+                detailUrl:
+                  'https://code.alibaba-inc.com/acme/widgets/codereview/42',
+                description: '',
+              },
+            });
+          }
+          return ''; // `auth whoami`
+        },
+      );
+      serveOrphanShape();
+    }
+
+    it('scopes an amend-orphaned anchor instead of refusing it', async () => {
+      serveAone();
+      const report = await reportFor({
+        since: ANCHOR,
+        host: 'gitlab.alibaba-inc.com',
+      });
+      expect(report.incremental).toEqual({
+        since: ANCHOR,
+        effective: true,
+        scope: SCOPE_A,
+        diffBase: BASE,
+      });
+      expect(report.diffPath).not.toBeNull();
+      // The published scope is the PR's own diff narrowed to the amend's
+      // delta — the untouched file is dropped, exactly as the GitHub
+      // incremental path narrows.
+      expect(writtenDiff()).toBe(NARROWED);
+      // ...and the diff came from the Aone ref namespace, not GitHub's.
+      expect(producerMocks.git.mock.calls).toContainEqual([
+        'fetch',
+        'origin',
+        'refs/merge-requests/42/head:qwen-review/pr-42',
+      ]);
+    });
+
+    it('never asks an ancestry question on the Aone platform', async () => {
+      serveAone();
+      await reportFor({ since: ANCHOR, host: 'gitlab.alibaba-inc.com' });
+      const ancestryCalls = producerMocks.gitOpt.mock.calls.filter(
+        (args) => args[0] === 'merge-base' && args[1] === '--is-ancestor',
+      );
+      // Neither the head test nor the clamp — a mutant re-asking either
+      // would refuse this anchor (every answer is "no") but could survive
+      // an outcome-only assertion, so pin the silence itself.
+      expect(ancestryCalls).toEqual([]);
+    });
+
+    it('keeps the ancestry tests on GitHub — an orphaned anchor there is a force-push', async () => {
+      // Same orphan shape, GitHub platform (no Aone host): the refusal
+      // stands, because on a push-based platform an anchor the head does
+      // not descend from is rewritten history, not an amend.
+      serveOrphanShape();
+      const report = await reportFor({ since: ANCHOR });
+      expect(report.incremental).toEqual({
+        since: ANCHOR,
+        effective: false,
+        reason: 'not-an-ancestor',
+      });
+      // The full range is the fallback.
+      expect(writtenDiff()).toBe(FULL_DIFF);
+    });
+  });
 });
 
 describe('resolveIncrementalAnchor', () => {
@@ -3031,6 +3113,112 @@ describe('resolveIncrementalAnchor', () => {
     });
     expect(r.diffBase).toBeNull();
   });
+
+  // ---- The AGit-Flow rule (design D7, #9618) ----------------------------
+  // Under AGit-Flow an update AMENDS the single CR commit in place, so the
+  // amended head has the cached head's parent, never the cached head itself
+  // — the anchor-behind-head test refuses EVERY update's anchor (the clamp
+  // fires only when the update also rebased). The Aone rule rules without
+  // ancestry: after the fetch both heads are local, and their diff is the
+  // update's delta.
+
+  it('noAncestry scopes an orphaned anchor the ancestry test refuses', () => {
+    // isAncestor answers "no" for everything — the exact amend shape. The
+    // constant-false probe also kills the mutant that keeps asking: were
+    // the test still consulted, its answer would refuse this anchor.
+    const r = resolveIncrementalAnchor(
+      ANCHOR,
+      HEAD,
+      probe({ isAncestor: () => false }),
+      null,
+      { noAncestry: true },
+    );
+    expect(r.incremental).toEqual({ since: ANCHOR, effective: true });
+    expect(r.diffBase).toBe(ANCHOR);
+  });
+
+  it('noAncestry asks NEITHER ancestry question — not the head test, not the clamp', () => {
+    // A constant-false isAncestor passes with a dropped guard on either
+    // check, so pin the ruling to silence: the merge base is present and
+    // "older" than nothing an amend can reach — the clamp's exact trigger
+    // after a rebase onto newer master.
+    const asked: Array<[string, string]> = [];
+    const r = resolveIncrementalAnchor(
+      ANCHOR,
+      HEAD,
+      probe({
+        isAncestor: (a, b) => {
+          asked.push([a, b]);
+          return false;
+        },
+      }),
+      { sha: 'c'.repeat(40), fetchFailed: false },
+      { noAncestry: true },
+    );
+    expect(r.incremental).toEqual({ since: ANCHOR, effective: true });
+    expect(r.diffBase).toBe(ANCHOR);
+    expect(asked).toEqual([]);
+  });
+
+  it('noAncestry keeps the existence refusals — a fresh clone cannot diff the orphan', () => {
+    // The ancestry skip is about LINEAGE, not presence: an anchor the
+    // object store does not hold (the round-1 fetch happened elsewhere)
+    // has no delta to capture, and says so with the deterministic reason.
+    const absent = resolveIncrementalAnchor(
+      ANCHOR,
+      HEAD,
+      probe({ commitExists: () => false, isAncestor: () => false }),
+      null,
+      { noAncestry: true },
+    );
+    expect(absent.incremental).toEqual({
+      since: ANCHOR,
+      effective: false,
+      reason: 'unknown-commit',
+    });
+    expect(absent.diffBase).toBeNull();
+    const unresolvable = resolveIncrementalAnchor(
+      ANCHOR,
+      HEAD,
+      probe({ resolveCommit: () => null, isAncestor: () => false }),
+      null,
+      { noAncestry: true },
+    );
+    expect(unresolvable.incremental.reason).toBe('unknown-commit');
+  });
+
+  it('noAncestry rules upToDate when the anchor IS the head', () => {
+    // A re-run with no amend in between: the identity comparison is not an
+    // ancestry test, so it rules exactly as on GitHub.
+    const r = resolveIncrementalAnchor(HEAD, HEAD, probe(), null, {
+      noAncestry: true,
+    });
+    expect(r.incremental).toEqual({
+      since: HEAD,
+      effective: true,
+      upToDate: true,
+    });
+    expect(r.diffBase).toBeNull();
+  });
+
+  it('noAncestry still refuses a possibly-stale base — base-untrusted is not an ancestry test', () => {
+    // The published scope is assembled from the base-derived full capture;
+    // a base the run flagged possibly-stale is a capture no sibling guard
+    // rules on, ancestry skip or not.
+    const r = resolveIncrementalAnchor(
+      ANCHOR,
+      HEAD,
+      probe({ isAncestor: () => false }),
+      { sha: 'c'.repeat(40), fetchFailed: true },
+      { noAncestry: true },
+    );
+    expect(r.incremental).toEqual({
+      since: ANCHOR,
+      effective: false,
+      reason: 'base-untrusted',
+    });
+    expect(r.diffBase).toBeNull();
+  });
 });
 
 describe('isEmptyDiff', () => {