fix: reject --base combined with --scope working-tree

scripts/lib/git.mjs

@@ -233,6 +233,20 @@ export function resolveReviewTarget(cwd, options = {}) {
   const state = getWorkingTreeState(cwd);
   const supportedScopes = new Set(["auto", "working-tree", "branch"]);
 
+  if (!supportedScopes.has(requestedScope)) {
+    throw new Error(
+      `Unsupported review scope "${requestedScope}". Use one of: auto, working-tree, branch, or pass --base <ref>.`
+    );
+  }
+
+  // A working-tree review always compares against HEAD, so a base ref would be
+  // ignored. Refuse the pair instead of silently reviewing a different diff.
+  if (baseRef && requestedScope === "working-tree") {
+    throw new Error(
+      `--base cannot be combined with --scope working-tree: a working-tree review compares against HEAD. Drop --base to review uncommitted changes, or drop --scope to review the branch against ${baseRef}.`
+    );
+  }
+
   if (baseRef) {
     return {
       mode: "branch",
@@ -250,12 +264,6 @@ export function resolveReviewTarget(cwd, options = {}) {
     };
   }
 
-  if (!supportedScopes.has(requestedScope)) {
-    throw new Error(
-      `Unsupported review scope "${requestedScope}". Use one of: auto, working-tree, branch, or pass --base <ref>.`
-    );
-  }
-
   if (requestedScope === "branch") {
     const detectedBase = detectDefaultBranch(cwd);
     return {

skills/adversarial-review/SKILL.md

@@ -53,6 +53,7 @@ Argument handling:
 - `$cc:adversarial-review` uses the same review target selection as `$cc:review`.
 - It supports working-tree review, branch review, and `--base <ref>`.
 - It does not support `--scope staged` or `--scope unstaged`.
+- Do not combine `--base <ref>` with `--scope working-tree`; the companion rejects that pair because a working-tree review always compares against `HEAD`.
 - Unlike `$cc:review`, it can still take extra focus text after the flags.
 - The companion review process itself always runs in the foreground. Background mode only changes how Codex launches that command.
 - For the detailed execution contract, treat the internal runtime reference at `../../internal-skills/review-runtime/runtime.md` as supporting guidance only. It is an internal reference document, not a public skill to invoke.

skills/review/SKILL.md

@@ -28,6 +28,7 @@ Rules:
 - Before launching the review, stay in read-only inspection mode: inspect git status and diff stats only, then ask at most one user question about whether to wait or run in background.
 - Preserve the user's review scope flags exactly.
 - Do not accept staged-only or unstaged-only review modes.
+- `--base <ref>` reviews the branch against that ref. Do not combine it with `--scope working-tree`; the companion rejects that pair because a working-tree review always compares against `HEAD`.
 - Do not add extra review instructions or focus text. Route those requests to `$cc:adversarial-review`.
 
 Execution mode rules:

tests/git.test.mjs

@@ -9,7 +9,7 @@ import os from "node:os";
 import path from "node:path";
 import { spawnSync } from "node:child_process";
 
-import { collectReviewContext, getWorkingTreeFingerprint } from "../scripts/lib/git.mjs";
+import { collectReviewContext, getWorkingTreeFingerprint, resolveReviewTarget } from "../scripts/lib/git.mjs";
 
 const tempRepos = [];
 
@@ -215,3 +215,52 @@ describe("collectReviewContext", () => {
     assert.equal(typeof fingerprint.unstagedDiffHash, "string");
   });
 });
+
+describe("resolveReviewTarget", () => {
+  function createRepoWithStagedChange() {
+    const repo = createRepo();
+    fs.writeFileSync(path.join(repo, "app.js"), "export const value = 1;\n", "utf8");
+    runGit(repo, ["add", "app.js"]);
+    runGit(repo, ["commit", "-m", "initial"]);
+    fs.writeFileSync(path.join(repo, "app.js"), "export const value = 2;\n", "utf8");
+    runGit(repo, ["add", "app.js"]);
+    return repo;
+  }
+
+  // A branch review of main against main is empty, so silently honoring --base
+  // here would report a clean review of uncommitted work it never looked at.
+  it("refuses --base with --scope working-tree instead of dropping the scope", () => {
+    const repo = createRepoWithStagedChange();
+
+    assert.throws(
+      () => resolveReviewTarget(repo, { base: "main", scope: "working-tree" }),
+      /--base cannot be combined with --scope working-tree/
+    );
+  });
+
+  it("rejects an unknown scope even when --base is given", () => {
+    const repo = createRepoWithStagedChange();
+
+    assert.throws(
+      () => resolveReviewTarget(repo, { base: "main", scope: "staged" }),
+      /Unsupported review scope "staged"/
+    );
+  });
+
+  it("still reviews the branch against --base with the auto or branch scope", () => {
+    const repo = createRepoWithStagedChange();
+
+    for (const scope of [undefined, "auto", "branch"]) {
+      const target = resolveReviewTarget(repo, { base: "main", scope });
+      assert.equal(target.mode, "branch");
+      assert.equal(target.baseRef, "main");
+    }
+  });
+
+  it("still reviews the working tree when only --scope working-tree is given", () => {
+    const repo = createRepoWithStagedChange();
+
+    const target = resolveReviewTarget(repo, { scope: "working-tree" });
+    assert.equal(target.mode, "working-tree");
+  });
+});