Reject missing review base refs

plugins/codex/scripts/codex-companion.mjs

@@ -262,7 +262,8 @@ function buildNativeReviewTarget(target) {
   }
 
   if (target.mode === "branch") {
-    return { type: "baseBranch", branch: target.baseRef };
+    const branch = target.nativeBaseRef ?? (target.baseRef.startsWith("-") ? target.baseCommit : target.baseRef);
+    return { type: "baseBranch", branch };
   }
 
   return null;

plugins/codex/scripts/lib/git.mjs

@@ -66,12 +66,12 @@ function measureCombinedGitOutputBytes(cwd, argSets, maxBytes) {
   return totalBytes;
 }
 
-function buildBranchComparison(cwd, baseRef) {
-  const mergeBase = gitChecked(cwd, ["merge-base", "HEAD", baseRef]).stdout.trim();
+function buildBranchComparison(cwd, baseCommit) {
+  const mergeBase = gitChecked(cwd, ["merge-base", "HEAD", baseCommit]).stdout.trim();
   return {
     mergeBase,
     commitRange: `${mergeBase}..HEAD`,
-    reviewRange: `${baseRef}...HEAD`
+    reviewRange: `${baseCommit}...HEAD`
   };
 }
 
@@ -91,30 +91,37 @@ export function getRepoRoot(cwd) {
   return gitChecked(cwd, ["rev-parse", "--show-toplevel"]).stdout.trim();
 }
 
-export function detectDefaultBranch(cwd) {
+function detectDefaultBranchTarget(cwd) {
   const symbolic = git(cwd, ["symbolic-ref", "refs/remotes/origin/HEAD"]);
   if (symbolic.status === 0) {
     const remoteHead = symbolic.stdout.trim();
     if (remoteHead.startsWith("refs/remotes/origin/")) {
-      return remoteHead.replace("refs/remotes/origin/", "");
+      return {
+        baseRef: remoteHead.replace("refs/remotes/origin/", ""),
+        commitRef: remoteHead
+      };
     }
   }
 
   const candidates = ["main", "master", "trunk"];
   for (const candidate of candidates) {
     const local = git(cwd, ["show-ref", "--verify", "--quiet", `refs/heads/${candidate}`]);
     if (local.status === 0) {
-      return candidate;
+      return { baseRef: candidate, commitRef: candidate };
     }
     const remote = git(cwd, ["show-ref", "--verify", "--quiet", `refs/remotes/origin/${candidate}`]);
     if (remote.status === 0) {
-      return `origin/${candidate}`;
+      return { baseRef: `origin/${candidate}`, commitRef: `refs/remotes/origin/${candidate}` };
     }
   }
 
   throw new Error("Unable to detect the repository default branch. Pass --base <ref> or use --scope working-tree.");
 }
 
+export function detectDefaultBranch(cwd) {
+  return detectDefaultBranchTarget(cwd).baseRef;
+}
+
 export function getCurrentBranch(cwd) {
   return gitChecked(cwd, ["branch", "--show-current"]).stdout.trim() || "HEAD";
 }
@@ -132,23 +139,45 @@ export function getWorkingTreeState(cwd) {
   };
 }
 
+function ensureCommitRef(cwd, baseRef) {
+  const resolved = git(cwd, ["rev-parse", "--verify", "--quiet", "--end-of-options", baseRef]);
+  if (resolved.error) {
+    throw resolved.error;
+  }
+  if (resolved.status !== 0) {
+    throw new Error(`base ${baseRef} not found in this repository`);
+  }
+
+  const commit = git(cwd, ["rev-parse", "--verify", "--quiet", "--end-of-options", `${resolved.stdout.trim()}^{commit}`]);
+  if (commit.error) {
+    throw commit.error;
+  }
+  if (commit.status !== 0) {
+    throw new Error(`base ${baseRef} not found in this repository`);
+  }
+  return commit.stdout.trim();
+}
+
 export function resolveReviewTarget(cwd, options = {}) {
   ensureGitRepository(cwd);
 
   const requestedScope = options.scope ?? "auto";
   const baseRef = options.base ?? null;
-  const state = getWorkingTreeState(cwd);
   const supportedScopes = new Set(["auto", "working-tree", "branch"]);
 
   if (baseRef) {
+    const baseCommit = ensureCommitRef(cwd, baseRef);
     return {
       mode: "branch",
       label: `branch diff against ${baseRef}`,
       baseRef,
+      baseCommit,
       explicit: true
     };
   }
 
+  const state = getWorkingTreeState(cwd);
+
   if (requestedScope === "working-tree") {
     return {
       mode: "working-tree",
@@ -164,11 +193,14 @@ export function resolveReviewTarget(cwd, options = {}) {
   }
 
   if (requestedScope === "branch") {
-    const detectedBase = detectDefaultBranch(cwd);
+    const detected = detectDefaultBranchTarget(cwd);
+    const baseCommit = ensureCommitRef(cwd, detected.commitRef);
     return {
       mode: "branch",
-      label: `branch diff against ${detectedBase}`,
-      baseRef: detectedBase,
+      label: `branch diff against ${detected.baseRef}`,
+      baseRef: detected.baseRef,
+      baseCommit,
+      nativeBaseRef: detected.baseRef.startsWith("-") ? baseCommit : detected.commitRef,
       explicit: true
     };
   }
@@ -181,11 +213,14 @@ export function resolveReviewTarget(cwd, options = {}) {
     };
   }
 
-  const detectedBase = detectDefaultBranch(cwd);
+  const detected = detectDefaultBranchTarget(cwd);
+  const baseCommit = ensureCommitRef(cwd, detected.commitRef);
   return {
     mode: "branch",
-    label: `branch diff against ${detectedBase}`,
-    baseRef: detectedBase,
+    label: `branch diff against ${detected.baseRef}`,
+    baseRef: detected.baseRef,
+    baseCommit,
+    nativeBaseRef: detected.baseRef.startsWith("-") ? baseCommit : detected.commitRef,
     explicit: false
   };
 }
@@ -261,7 +296,7 @@ function collectWorkingTreeContext(cwd, state, options = {}) {
 
 function collectBranchContext(cwd, baseRef, options = {}) {
   const includeDiff = options.includeDiff !== false;
-  const comparison = options.comparison ?? buildBranchComparison(cwd, baseRef);
+  const comparison = options.comparison ?? buildBranchComparison(cwd, ensureCommitRef(cwd, baseRef));
   const currentBranch = getCurrentBranch(cwd);
   const changedFiles = gitChecked(cwd, ["diff", "--name-only", comparison.commitRange]).stdout.trim().split("\n").filter(Boolean);
   const logOutput = gitChecked(cwd, ["log", "--oneline", "--decorate", comparison.commitRange]).stdout.trim();
@@ -322,7 +357,8 @@ export function collectReviewContext(cwd, target, options = {}) {
         diffBytes <= maxInlineDiffBytes);
     details = collectWorkingTreeContext(repoRoot, state, { includeDiff });
   } else {
-    const comparison = buildBranchComparison(repoRoot, target.baseRef);
+    const baseCommit = target.baseCommit ?? ensureCommitRef(repoRoot, target.baseRef);
+    const comparison = buildBranchComparison(repoRoot, baseCommit);
     const fileCount = gitChecked(repoRoot, ["diff", "--name-only", comparison.commitRange]).stdout.trim().split("\n").filter(Boolean).length;
     diffBytes = measureGitOutputBytes(
       repoRoot,

tests/git.test.mjs

@@ -84,6 +84,75 @@ test("resolveReviewTarget honors explicit base overrides", () => {
   assert.equal(target.baseRef, "main");
 });
 
+test("resolveReviewTarget accepts commit-message searches as explicit bases", () => {
+  const cwd = makeTempDir();
+  initGitRepo(cwd);
+  fs.writeFileSync(path.join(cwd, "app.js"), "console.log('base');\n");
+  run("git", ["add", "app.js"], { cwd });
+  run("git", ["commit", "-m", "release baseline"], { cwd, shell: false });
+  const baseCommit = run("git", ["rev-parse", "HEAD"], { cwd, shell: false }).stdout.trim();
+  run("git", ["checkout", "-b", "feature/test"], { cwd });
+  fs.writeFileSync(path.join(cwd, "app.js"), "console.log('feature');\n");
+  run("git", ["add", "app.js"], { cwd });
+  run("git", ["commit", "-m", "feature"], { cwd });
+
+  const baseRef = ":/release baseline";
+  const target = resolveReviewTarget(cwd, { base: baseRef });
+  const context = collectReviewContext(cwd, target);
+
+  assert.equal(target.mode, "branch");
+  assert.equal(target.baseRef, baseRef);
+  assert.equal(target.baseCommit, baseCommit);
+  assert.equal(context.comparison.mergeBase, baseCommit);
+  assert.equal(context.comparison.reviewRange, `${baseCommit}...HEAD`);
+  assert.deepEqual(context.changedFiles, ["app.js"]);
+  assert.match(context.summary, /against :\/release baseline/);
+  assert.match(context.content, /-console\.log\('base'\);/);
+  assert.match(context.content, /\+console\.log\('feature'\);/);
+});
+
+test("collectReviewContext uses the canonical commit for dash-leading explicit bases", () => {
+  const cwd = makeTempDir();
+  initGitRepo(cwd);
+  fs.writeFileSync(path.join(cwd, "app.js"), "console.log('base');\n");
+  run("git", ["add", "app.js"], { cwd });
+  run("git", ["commit", "-m", "base"], { cwd });
+  const baseCommit = run("git", ["rev-parse", "HEAD"], { cwd, shell: false }).stdout.trim();
+  run("git", ["update-ref", "refs/heads/-release-baseline", baseCommit], { cwd, shell: false });
+  run("git", ["checkout", "-b", "feature/test"], { cwd });
+  fs.writeFileSync(path.join(cwd, "app.js"), "console.log('feature');\n");
+  run("git", ["add", "app.js"], { cwd });
+  run("git", ["commit", "-m", "feature"], { cwd });
+
+  const baseRef = "-release-baseline";
+  const target = resolveReviewTarget(cwd, { base: baseRef });
+  const context = collectReviewContext(cwd, target);
+
+  assert.equal(target.baseRef, baseRef);
+  assert.equal(target.baseCommit, baseCommit);
+  assert.equal(context.comparison.mergeBase, baseCommit);
+  assert.equal(context.comparison.reviewRange, `${baseCommit}...HEAD`);
+  assert.deepEqual(context.changedFiles, ["app.js"]);
+  assert.match(context.summary, /against -release-baseline/);
+});
+
+test("resolveReviewTarget rejects missing or non-commit explicit bases", () => {
+  const cwd = makeTempDir();
+  initGitRepo(cwd);
+  fs.writeFileSync(path.join(cwd, "app.js"), "console.log('v1');\n");
+  run("git", ["add", "app.js"], { cwd });
+  run("git", ["commit", "-m", "init"], { cwd });
+
+  assert.throws(
+    () => resolveReviewTarget(cwd, { base: "missing-base" }),
+    /base missing-base not found in this repository/
+  );
+  assert.throws(
+    () => resolveReviewTarget(cwd, { base: "HEAD:app.js" }),
+    /base HEAD:app.js not found in this repository/
+  );
+});
+
 test("resolveReviewTarget requires an explicit base when no default branch can be inferred", () => {
   const cwd = makeTempDir();
   initGitRepo(cwd);

tests/runtime.test.mjs

@@ -356,16 +356,122 @@ test("review accepts the quoted raw argument style for built-in base-branch revi
   run("git", ["commit", "-m", "init"], { cwd: repo });
   fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 2;\n");
 
-  const result = run("node", [SCRIPT, "review", "--base main"], {
+  const result = run(process.execPath, [SCRIPT, "review", "--base main"], {
     cwd: repo,
     env: buildEnv(binDir)
   });
 
-  assert.equal(result.status, 0);
+  assert.equal(result.status, 0, result.stderr);
   assert.match(result.stdout, /Reviewed changes against main/);
   assert.match(result.stdout, /No material issues found/);
 });
 
+test("review sends dash-leading explicit bases to app-server as canonical commits", () => {
+  const repo = makeTempDir();
+  const binDir = makeTempDir();
+  installFakeCodex(binDir);
+  initGitRepo(repo);
+  fs.writeFileSync(path.join(repo, "app.js"), "console.log('base');\n");
+  run("git", ["add", "app.js"], { cwd: repo });
+  run("git", ["commit", "-m", "base"], { cwd: repo });
+  const baseCommit = run("git", ["rev-parse", "HEAD"], { cwd: repo, shell: false }).stdout.trim();
+  run("git", ["update-ref", "refs/heads/-release-baseline", baseCommit], { cwd: repo, shell: false });
+  run("git", ["checkout", "-b", "feature/test"], { cwd: repo });
+  fs.writeFileSync(path.join(repo, "app.js"), "console.log('feature');\n");
+  run("git", ["add", "app.js"], { cwd: repo });
+  run("git", ["commit", "-m", "feature"], { cwd: repo });
+
+  const result = run(process.execPath, [SCRIPT, "review", "--base", "-release-baseline"], {
+    cwd: repo,
+    env: buildEnv(binDir)
+  });
+
+  assert.equal(result.status, 0, result.stderr);
+  assert.match(result.stdout, /branch diff against -release-baseline/i);
+  assert.match(result.stdout, new RegExp(`Reviewed changes against ${baseCommit}\\.`));
+});
+
+test("review sends dash-leading detected bases to app-server as canonical commits", () => {
+  const repo = makeTempDir();
+  const binDir = makeTempDir();
+  installFakeCodex(binDir);
+  initGitRepo(repo);
+  fs.writeFileSync(path.join(repo, "app.js"), "console.log('base');\n");
+  run("git", ["add", "app.js"], { cwd: repo });
+  run("git", ["commit", "-m", "base"], { cwd: repo });
+  const baseCommit = run("git", ["rev-parse", "HEAD"], { cwd: repo, shell: false }).stdout.trim();
+  run("git", ["update-ref", "refs/remotes/origin/-release-baseline", baseCommit], { cwd: repo, shell: false });
+  run("git", ["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/-release-baseline"], {
+    cwd: repo,
+    shell: false
+  });
+  run("git", ["checkout", "-b", "feature/test"], { cwd: repo });
+  fs.writeFileSync(path.join(repo, "app.js"), "console.log('feature');\n");
+  run("git", ["add", "app.js"], { cwd: repo });
+  run("git", ["commit", "-m", "feature"], { cwd: repo });
+
+  const result = run(process.execPath, [SCRIPT, "review", "--scope", "branch"], {
+    cwd: repo,
+    env: buildEnv(binDir)
+  });
+
+  assert.equal(result.status, 0, result.stderr);
+  assert.match(result.stdout, /branch diff against -release-baseline/i);
+  assert.match(result.stdout, new RegExp(`Reviewed changes against ${baseCommit}\\.`));
+});
+
+test("review preserves symbolic remote defaults without local branches", () => {
+  const repo = makeTempDir();
+  const binDir = makeTempDir();
+  installFakeCodex(binDir);
+  initGitRepo(repo);
+  fs.writeFileSync(path.join(repo, "app.js"), "console.log('base');\n");
+  run("git", ["add", "app.js"], { cwd: repo });
+  run("git", ["commit", "-m", "base"], { cwd: repo });
+  const baseCommit = run("git", ["rev-parse", "HEAD"], { cwd: repo, shell: false }).stdout.trim();
+  run("git", ["update-ref", "refs/remotes/origin/main", baseCommit], { cwd: repo, shell: false });
+  run("git", ["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/main"], {
+    cwd: repo,
+    shell: false
+  });
+  run("git", ["checkout", "--detach"], { cwd: repo, shell: false });
+  run("git", ["branch", "-D", "main"], { cwd: repo, shell: false });
+  fs.writeFileSync(path.join(repo, "app.js"), "console.log('feature');\n");
+  run("git", ["add", "app.js"], { cwd: repo });
+  run("git", ["commit", "-m", "feature"], { cwd: repo });
+
+  for (const args of [["review", "--scope", "branch"], ["review"]]) {
+    const result = run(process.execPath, [SCRIPT, ...args], {
+      cwd: repo,
+      env: buildEnv(binDir)
+    });
+
+    assert.equal(result.status, 0, result.stderr);
+    assert.match(result.stdout, /branch diff against main/i);
+    assert.match(result.stdout, /Reviewed changes against refs\/remotes\/origin\/main\./);
+  }
+});
+
+test("review rejects a missing explicit base before starting Codex", () => {
+  const repo = makeTempDir();
+  const binDir = makeTempDir();
+  const fakeCodexState = path.join(binDir, "fake-codex-state.json");
+  installFakeCodex(binDir);
+  initGitRepo(repo);
+  fs.writeFileSync(path.join(repo, "README.md"), "hello\n");
+  run("git", ["add", "README.md"], { cwd: repo });
+  run("git", ["commit", "-m", "init"], { cwd: repo });
+
+  const result = run(process.execPath, [SCRIPT, "review", "--base", "missing-base"], {
+    cwd: repo,
+    env: buildEnv(binDir)
+  });
+
+  assert.notEqual(result.status, 0);
+  assert.match(result.stderr, /base missing-base not found in this repository/);
+  assert.equal(fs.existsSync(fakeCodexState), false);
+});
+
 test("adversarial review renders structured findings over app-server turn/start", () => {
   const repo = makeTempDir();
   const binDir = makeTempDir();