feat(review): add a --topology minimal single-pass A/B arm

packages/cli/src/commands/review/lib/authorization.ts

@@ -264,6 +264,18 @@ function lookupRecordedHost(
   return { host: undefined, unbound: false };
 }
 
+/**
+ * The structural class of a write refusal. The advice at the submit call
+ * site branches on THIS, never on the refusal text: `why` embeds the
+ * operator's verbatim recorded arguments (JSON.stringify of the raw
+ * record), and any marker string can itself appear inside that quoted
+ * record — text that embeds operator input cannot classify itself.
+ */
+export type ReviewWriteRefusalClass =
+  | 'topology'
+  | 'comment-not-requested'
+  | 'unbound';
+
 /**
  * Exactly three things authorise a public write, and all are facts rather than
  * impressions: `--comment` in the arguments the user typed (re-parsed from the
@@ -275,6 +287,12 @@ function lookupRecordedHost(
 export function reviewWriteAuthorization(req: WriteAuthorizationRequest): {
   ok: boolean;
   why: string;
+  /**
+   * The refusal class — present on every refusal, absent on success. See
+   * `ReviewWriteRefusalClass` for why the caller branches on this instead
+   * of matching `why`.
+   */
+  cls?: ReviewWriteRefusalClass;
   /**
    * The host the recorded target names, when it names one: a pr-url target
    * carries it; a bare pr-number supplies a recorded `--host` flag or none.
@@ -343,6 +361,7 @@ export function reviewWriteAuthorization(req: WriteAuthorizationRequest): {
     // a plain re-run of the review fixes that — typing `--comment` does not.
     return {
       ok: false,
+      cls: req.defaultComment === true ? 'unbound' : 'comment-not-requested',
       why:
         req.defaultComment === true
           ? `no review arguments were recorded at ${path}, so no recorded ` +
@@ -354,30 +373,38 @@ export function reviewWriteAuthorization(req: WriteAuthorizationRequest): {
   }
 
   const verdict = parseReviewArgs(raw, { comment: req.defaultComment });
-  if (!verdict.comment.effective) {
-    // The refusal must name the REAL blocker. When comment was requested —
-    // by the flag or the standing `review.comment` setting — but the target
-    // is not a PR, effective is false because the arguments name no pull
-    // request to bind the write to; blaming a missing `--comment` flag the
-    // operator never typed (and implying typing one would fix it) misdirects.
+  if (!verdict.comment.effective && verdict.topology !== 'minimal') {
+    // When comment was requested — by the flag or the standing
+    // `review.comment` setting — but the target is not a PR, effective is
+    // false because the arguments name no pull request to bind the write to;
+    // blaming a missing `--comment` flag the operator never typed (and
+    // implying typing one would fix it) misdirects.
     const commentRequested =
       verdict.comment.requested || req.defaultComment === true;
     return {
       ok: false,
+      cls: commentRequested ? 'unbound' : 'comment-not-requested',
       why: commentRequested
         ? `the review arguments (${JSON.stringify(raw.trim())}) do not name a ` +
           'pull request, so they cannot authorise posting to one'
         : '`--comment` was not in the review arguments ' +
           `(${JSON.stringify(raw.trim())})`,
     };
   }
+  // A minimal record falls through to the binding checks below: the refusal
+  // must name the REAL blocker, and the topology is it only when it is the
+  // SOLE one — "re-run the review without it" cannot lift a refusal that a
+  // non-PR target, or another PR's number, repo, or host, still holds, and
+  // leading with the topology sends the operator to re-run into the same
+  // refusal with the binding blocker still unnamed.
 
   const t = verdict.target;
   const authorisedPr =
     t.type === 'pr-number' || t.type === 'pr-url' ? t.number : undefined;
   if (authorisedPr === undefined) {
     return {
       ok: false,
+      cls: 'unbound',
       why:
         `the review arguments (${JSON.stringify(raw.trim())}) do not name a ` +
         'pull request, so they cannot authorise posting to one',
@@ -386,6 +413,7 @@ export function reviewWriteAuthorization(req: WriteAuthorizationRequest): {
   if (authorisedPr !== req.pr) {
     return {
       ok: false,
+      cls: 'unbound',
       why:
         `the review arguments authorise pull request #${authorisedPr}, but ` +
         `this submission targets #${req.pr}`,
@@ -397,6 +425,7 @@ export function reviewWriteAuthorization(req: WriteAuthorizationRequest): {
       if (authorisedRepo.toLowerCase() !== req.repo.toLowerCase()) {
         return {
           ok: false,
+          cls: 'unbound',
           why:
             `the review arguments authorise ${authorisedRepo}, but this ` +
             `submission targets ${req.repo}`,
@@ -423,13 +452,30 @@ export function reviewWriteAuthorization(req: WriteAuthorizationRequest): {
     if (!hostUnasserted && !hostsEquivalent(t.host.toLowerCase(), writeHost)) {
       return {
         ok: false,
+        cls: 'unbound',
         why:
           `the review arguments authorise ${t.host}, but this submission ` +
           `targets ${req.host ?? 'github.com'}`,
       };
     }
   }
 
+  if (!verdict.comment.effective) {
+    // Minimal, and bound to this write on every axis above. When a comment
+    // source was recorded, the parser forced effective false, so the
+    // topology is the sole blocker and its remedy lifts the refusal; when
+    // none was, the topology is still the blocker to name — even a typed
+    // --comment would not lift the refusal while minimal stands.
+    return {
+      ok: false,
+      cls: 'topology',
+      why:
+        `the review arguments (${JSON.stringify(raw.trim())}) ran with ` +
+        '`--topology minimal`, which is terminal-only and cannot authorise ' +
+        'posting — re-run the review without it',
+    };
+  }
+
   return {
     ok: true,
     why: verdict.comment.requested

packages/cli/src/commands/review/parse-args.test.ts

@@ -809,6 +809,199 @@ describe('parseReviewArgs — --severity-floor (the convergence posture knob)',
   });
 });
 
+describe('parseReviewArgs — --topology (the minimal-prompt A/B arm)', () => {
+  it('defaults to auto: the standing effort-driven pipeline', () => {
+    const got = parseReviewArgs('6711');
+    expect(got.topology).toBe('auto');
+    expect(got.topologySource).toBe('default');
+  });
+
+  it('parses both forms case-insensitively; the last valid occurrence wins', () => {
+    expect(parseReviewArgs('6711 --topology minimal')).toMatchObject({
+      topology: 'minimal',
+      topologySource: 'explicit',
+    });
+    expect(parseReviewArgs('6711 --topology=Minimal')).toMatchObject({
+      topology: 'minimal',
+    });
+    expect(
+      parseReviewArgs('6711 --topology minimal --topology auto'),
+    ).toMatchObject({ topology: 'auto', topologySource: 'explicit' });
+  });
+
+  it('an explicit --topology auto is explicit, not the default', () => {
+    const got = parseReviewArgs('6711 --topology auto');
+    expect(got.topology).toBe('auto');
+    expect(got.topologySource).toBe('explicit');
+  });
+
+  it('selecting minimal does not change the target', () => {
+    expect(parseReviewArgs('6711 --topology minimal').target).toEqual({
+      type: 'pr-number',
+      number: 6711,
+    });
+    expect(parseReviewArgs('src/foo.ts --topology minimal').target).toEqual({
+      type: 'file',
+      path: 'src/foo.ts',
+    });
+  });
+
+  it('minimal gates --comment: terminal-only, posts nothing', () => {
+    const got = parseReviewArgs('6711 --topology minimal --comment');
+    expect(got.comment.requested).toBe(true);
+    expect(got.comment.effective).toBe(false);
+    expect(
+      got.warnings.some(
+        (w) => w.includes('`--comment`') && w.includes('terminal-only'),
+      ),
+    ).toBe(true);
+  });
+
+  it('minimal gates --fix: terminal-only, edits nothing', () => {
+    const got = parseReviewArgs('src/foo.ts --topology minimal --fix');
+    expect(got.fix.requested).toBe(true);
+    expect(got.fix.effective).toBe(false);
+    expect(
+      got.warnings.some(
+        (w) => w.includes('`--fix`') && w.includes('terminal-only'),
+      ),
+    ).toBe(true);
+  });
+
+  it('minimal gates --resume: a fresh single pass cannot continue an interrupted run', () => {
+    // The third flag the minimal arm gates: an effective resume would make
+    // `fetch-pr --resume` consume an interrupted pipeline run's lease and
+    // worktree for a pass that never continues it — destroying resumable
+    // state instead of either continuing or leaving it alone.
+    const got = parseReviewArgs('6711 --topology minimal --resume');
+    expect(got.resume.requested).toBe(true);
+    expect(got.resume.effective).toBe(false);
+    expect(
+      got.warnings.some(
+        (w) => w.includes('`--resume`') && w.includes('--topology minimal'),
+      ),
+    ).toBe(true);
+  });
+
+  it('an invalid value warns naming what is in effect, and never eats the target', () => {
+    const got = parseReviewArgs('--topology minial 6711');
+    expect(got.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(got.topology).toBe('auto');
+    expect(
+      got.warnings.some(
+        (w) =>
+          w.includes('Invalid --topology value "minial"') &&
+          w.includes('default topology'),
+      ),
+    ).toBe(true);
+  });
+
+  it('an invalid equals-form value warns instead of vanishing', () => {
+    const got = parseReviewArgs('6711 --topology=minial');
+    expect(got.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(got.topology).toBe('auto');
+    expect(
+      got.warnings.some((w) => w.includes('Invalid --topology value "minial"')),
+    ).toBe(true);
+  });
+
+  it('a sole invalid value becomes the target, and the warning says so', () => {
+    const got = parseReviewArgs('--topology minial');
+    expect(got.target).toEqual({ type: 'file', path: 'minial' });
+    expect(
+      got.warnings.some(
+        (w) =>
+          w.includes('Invalid --topology value "minial"') &&
+          w.includes('treating it as the review target'),
+      ),
+    ).toBe(true);
+  });
+
+  it('a PR-shaped value is rescued as the target, not discarded', () => {
+    // `--topology 6711` (forgot the value) must review PR 6711, not silently
+    // fall back to the local diff — the same rescue --effort/--severity-floor get.
+    const got = parseReviewArgs('--topology 6711');
+    expect(got.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(got.topology).toBe('auto');
+  });
+
+  it('minimal does not force effort the way --comment does', () => {
+    // minimal is terminal-only, so the comment-forces-high rule never fires;
+    // a local target's effort stays at its default.
+    const got = parseReviewArgs('src/foo.ts --topology minimal');
+    expect(got.effort).toBe('medium');
+    expect(got.effortSource).toBe('default');
+  });
+
+  it('the equals form rescues a PR-shaped value exactly as the spaced form does', () => {
+    // Sibling probes pin this for --effort/--severity-floor (the round-8
+    // regression); the topology copy must not diverge. Deleting the
+    // equals-form rescue branch reviews the local tree instead of PR 6711.
+    expect(parseReviewArgs('--topology=6711').target).toEqual({
+      type: 'pr-number',
+      number: 6711,
+    });
+  });
+
+  it('a quoted-empty value is consumed as missing, never an empty-string target', () => {
+    // Deleting the consumption branch leaves '' as the sole candidate, and
+    // it classifies as an empty-string file target.
+    const bare = parseReviewArgs('--topology ""');
+    expect(bare.target).toEqual({ type: 'local' });
+    expect(
+      bare.warnings.some((w) => w.includes('--topology requires a value')),
+    ).toBe(true);
+
+    const afterTarget = parseReviewArgs('6711 --topology ""');
+    expect(afterTarget.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(
+      afterTarget.warnings.some((w) =>
+        w.includes('--topology requires a value'),
+      ),
+    ).toBe(true);
+  });
+
+  it('flag-final or flag-followed is a missing value, never a consumed flag', () => {
+    // Deleting the branch eats the following token into the kept pool, so
+    // `--comment` never registers.
+    const flagFinal = parseReviewArgs('6711 --topology');
+    expect(flagFinal.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(flagFinal.topology).toBe('auto');
+    expect(
+      flagFinal.warnings.some((w) => w.includes('--topology requires a value')),
+    ).toBe(true);
+
+    const followed = parseReviewArgs('6711 --topology --comment');
+    expect(followed.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(followed.comment.requested).toBe(true);
+    expect(followed.topology).toBe('auto');
+    expect(
+      followed.warnings.some((w) => w.includes('--topology requires a value')),
+    ).toBe(true);
+  });
+
+  it('minimal gates the review.comment setting too, and the warning names it', () => {
+    // The suppression gate is written over the SETTING-OR-FLAG request, so a
+    // settings-driven comment is gated exactly like a flagged one — pinning
+    // `effective: false` here witnesses the gate itself: narrowing it to the
+    // flag alone would let the terminal-only arm post while every flag-based
+    // test stays green. And the warning must name the setting, not a flag
+    // the operator never typed — the forced-by-comment warning makes the
+    // same distinction.
+    const got = parseReviewArgs('6711 --topology minimal', { comment: true });
+    expect(got.comment.effective).toBe(false);
+    expect(
+      got.warnings.some(
+        (w) =>
+          w.includes('`review.comment` setting') && w.includes('terminal-only'),
+      ),
+    ).toBe(true);
+    expect(got.warnings.some((w) => w.includes('`--comment` is ignored'))).toBe(
+      false,
+    );
+  });
+});
+
 describe('parseReviewArgs — settings-provided defaults', () => {
   it('applies the configured effort when --effort is absent', () => {
     const got = parseReviewArgs('6711', { effort: 'medium' });
@@ -1155,6 +1348,19 @@ describe('parseArgsCommand wiring', () => {
     expect(written).toBe(String(vi.mocked(writeStdoutLine).mock.calls[0][0]));
   });
 
+  it('--topology minimal survives the stdin → yargs → handler path', async () => {
+    // The flag must reach the printed verdict through the real handler, not
+    // just the pure function: a wiring drop leaves every pure-function test
+    // green while real `/review … --topology minimal` runs the full pipeline.
+    fsState.stdin = '6711 --topology minimal --comment\n';
+    await runCli(['parse-args', '--stdin']);
+    const got = printedVerdict();
+    expect(got.target).toEqual({ type: 'pr-number', number: 6711 });
+    expect(got.topology).toBe('minimal');
+    expect(got.topologySource).toBe('explicit');
+    expect(got.comment).toEqual({ requested: true, effective: false });
+  });
+
   // The real CLI nests this command under `review`, which changes what
   // yargs puts in argv._ (['review', 'parse-args'] instead of
   // ['parse-args']) — the smuggle guard once read that command path as