refactor(review): move review policy behind VCS providers

apps/hook/server/index.ts

@@ -94,8 +94,8 @@ import {
   startGoalSetupServer,
   handleGoalSetupServerReady,
 } from "@plannotator/server/goal-setup";
-import { type DiffType, detectManagedVcs, prepareLocalReviewDiff, gitRuntime } from "@plannotator/server/vcs";
-import { loadConfig, resolveDefaultDiffType, resolveSharingEnabled } from "@plannotator/shared/config";
+import { type DiffType, type VcsProvider, detectManagedVcs, prepareLocalReviewDiff, resolveConfiguredVcsReviewDefault, gitRuntime } from "@plannotator/server/vcs";
+import { loadConfig, resolveSharingEnabled } from "@plannotator/shared/config";
 import { parseReviewArgs, type ParsedReviewArgs } from "@plannotator/shared/review-args";
 import { resolveReviewOpenState, type ReviewOpenState } from "@plannotator/shared/review-open-state";
 import { listBranches, type AvailableBranches } from "@plannotator/shared/review-core";
@@ -368,7 +368,7 @@ async function resolveCliReviewOpenState(
   options: {
     isPRMode: boolean;
     isWorkspace: boolean;
-    providerId?: "git" | "gitbutler" | "jj" | "p4";
+    provider?: VcsProvider;
     resolvedDefaultDiffType: DiffType;
     cwd?: string;
   },
@@ -382,7 +382,7 @@ async function resolveCliReviewOpenState(
     reviewArgs.base !== undefined &&
     !options.isPRMode &&
     !options.isWorkspace &&
-    options.providerId === "git"
+    options.provider?.id === "git"
   ) {
     // The probe is the whole point of CLI-side resolution: without it a
     // typo'd base produces a confidently-mislabelled merge-base→HEAD diff
@@ -403,7 +403,9 @@ async function resolveCliReviewOpenState(
     parsed: reviewArgs,
     isPRMode: options.isPRMode,
     isWorkspace: options.isWorkspace,
-    providerId: options.providerId,
+    provider: options.provider
+      ? { resolve: options.provider.reviewPolicy.resolveOpenState }
+      : undefined,
     resolvedDefaultDiffType: options.resolvedDefaultDiffType,
     baseResolves,
     availableBranches,
@@ -837,7 +839,7 @@ if (args[0] === "sessions") {
     await resolveCliReviewOpenState(reviewArgs, {
       isPRMode: true,
       isWorkspace: false,
-      resolvedDefaultDiffType: resolveDefaultDiffType(loadConfig()),
+      resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(loadConfig()),
     });
     const prRef = parsePRUrl(urlArg);
     if (!prRef) {
@@ -1096,13 +1098,14 @@ if (args[0] === "sessions") {
         isPRMode: false,
         isWorkspace: false,
         providerId,
-        resolvedDefaultDiffType: resolveDefaultDiffType(config),
+        provider: managedVcs ?? undefined,
+        resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
       });
       const diffResult = await prepareLocalReviewDiff({
         vcsType: reviewArgs.vcsType,
-        requestedDiffType: openState.requestedDiffType,
+        requestedDiffType: openState.requestedDiffType as DiffType | undefined,
         requestedBase: openState.requestedBase,
-        configuredDiffType: resolveDefaultDiffType(config),
+        configuredDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
         hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
       });
       gitContext = diffResult.gitContext;
@@ -1121,10 +1124,10 @@ if (args[0] === "sessions") {
       await resolveCliReviewOpenState(reviewArgs, {
         isPRMode: false,
         isWorkspace: true,
-        resolvedDefaultDiffType: resolveDefaultDiffType(config),
+        resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config),
       });
       workspace = await buildLocalWorkspaceReview(process.cwd(), {
-        configuredDiffType: resolveDefaultDiffType(config),
+        configuredDiffType: resolveConfiguredVcsReviewDefault(config),
         hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
       });
       if (workspace.repos.length === 0) {
@@ -1856,7 +1859,7 @@ if (args[0] === "sessions") {
     await resolveCliReviewOpenState(reviewArgs, {
       isPRMode: true,
       isWorkspace: false,
-      resolvedDefaultDiffType: resolveDefaultDiffType(loadConfig()),
+      resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(loadConfig()),
     });
     const prRef = parsePRUrl(urlArg);
     if (!prRef) {
@@ -1902,16 +1905,16 @@ if (args[0] === "sessions") {
       const openState = await resolveCliReviewOpenState(reviewArgs, {
         isPRMode: false,
         isWorkspace: false,
-        providerId,
-        resolvedDefaultDiffType: resolveDefaultDiffType(config),
+        provider: managedVcs ?? undefined,
+        resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
         cwd,
       });
       const diffResult = await prepareLocalReviewDiff({
         cwd,
         vcsType: reviewArgs.vcsType,
-        requestedDiffType: openState.requestedDiffType,
+        requestedDiffType: openState.requestedDiffType as DiffType | undefined,
         requestedBase: openState.requestedBase,
-        configuredDiffType: resolveDefaultDiffType(config),
+        configuredDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
         hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
       });
       gitContext = diffResult.gitContext;
@@ -1925,11 +1928,11 @@ if (args[0] === "sessions") {
       await resolveCliReviewOpenState(reviewArgs, {
         isPRMode: false,
         isWorkspace: true,
-        resolvedDefaultDiffType: resolveDefaultDiffType(config),
+        resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config),
         cwd,
       });
       workspace = await buildLocalWorkspaceReview(cwd, {
-        configuredDiffType: resolveDefaultDiffType(config),
+        configuredDiffType: resolveConfiguredVcsReviewDefault(config),
         hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
       });
       if (workspace.repos.length === 0) {

apps/opencode-plugin/commands.ts

@@ -13,11 +13,11 @@ import {
   startAnnotateServer,
   handleAnnotateServerReady,
 } from "@plannotator/server/annotate";
-import { type DiffType, prepareLocalReviewDiff, detectManagedVcs, gitRuntime } from "@plannotator/server/vcs";
+import { type DiffType, prepareLocalReviewDiff, detectManagedVcs, resolveConfiguredVcsReviewDefault, gitRuntime } from "@plannotator/server/vcs";
 import { resolveReviewOpenState } from "@plannotator/shared/review-open-state";
 import { detectProjectName } from "@plannotator/server/project";
 import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getMRLabel, getMRNumberLabel, getDisplayRepo } from "@plannotator/server/pr";
-import { loadConfig, resolveDefaultDiffType, resolveUseJina } from "@plannotator/shared/config";
+import { loadConfig, resolveUseJina } from "@plannotator/shared/config";
 import {
   composeReviewApprovedMessage,
   getAnnotateApprovedWithNotesPrompt,
@@ -102,7 +102,7 @@ export async function handleReviewCommand(
         parsed: reviewArgs,
         isPRMode: true,
         isWorkspace: false,
-        resolvedDefaultDiffType: resolveDefaultDiffType(loadConfig()),
+        resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(loadConfig()),
       });
       if (openState.error) {
         client.app.log({ level: "error", message: `[Plannotator] ${openState.error}` });
@@ -162,8 +162,10 @@ export async function handleReviewCommand(
         parsed: reviewArgs,
         isPRMode: false,
         isWorkspace: false,
-        providerId,
-        resolvedDefaultDiffType: resolveDefaultDiffType(config),
+        provider: managedVcs
+          ? { resolve: managedVcs.reviewPolicy.resolveOpenState }
+          : undefined,
+        resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
         baseResolves,
       });
       if (openState.error) {
@@ -177,9 +179,9 @@ export async function handleReviewCommand(
         const diffResult = await prepareLocalReviewDiff({
           cwd,
           vcsType: reviewArgs.vcsType,
-          requestedDiffType: openState.requestedDiffType,
+          requestedDiffType: openState.requestedDiffType as DiffType | undefined,
           requestedBase: openState.requestedBase,
-          configuredDiffType: resolveDefaultDiffType(config),
+          configuredDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
           hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
         });
         gitContext = diffResult.gitContext;
@@ -202,15 +204,15 @@ export async function handleReviewCommand(
           parsed: reviewArgs,
           isPRMode: false,
           isWorkspace: true,
-          resolvedDefaultDiffType: resolveDefaultDiffType(config),
+          resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config),
         });
         if (openState.error) {
           client.app.log({ level: "error", message: `[Plannotator] ${openState.error}` });
           return;
         }
       }
       workspace = await buildLocalWorkspaceReview(cwd, {
-        configuredDiffType: resolveDefaultDiffType(config),
+        configuredDiffType: resolveConfiguredVcsReviewDefault(config),
         hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
       });
       if (workspace.repos.length === 0) {

apps/pi-extension/plannotator-browser.ts

@@ -6,9 +6,11 @@ import { createWorktreePool, type WorktreePool } from "./generated/worktree-pool
 import type { ExtensionContext } from "@earendil-works/pi-coding-agent";
 import {
 	prepareLocalReviewDiff,
+	resolveConfiguredVcsReviewDefault,
 	reviewRuntime,
 	detectManagedVcs,
 	getVcsContext,
+	getVcsReviewPolicy,
 	getVcsDiffFingerprint,
 	getVcsFileContentsForDiff,
 	canStageFiles,
@@ -34,7 +36,7 @@ import {
 } from "./generated/pr-provider.ts";
 import { parseRemoteUrl } from "./generated/repo.ts";
 import { fetchRef, createWorktree, removeWorktree, ensureObjectAvailable } from "./generated/worktree.ts";
-import { loadConfig, resolveDefaultDiffType, resolveSharingEnabled } from "./generated/config.ts";
+import { loadConfig, resolveSharingEnabled } from "./generated/config.ts";
 import {
 	WorkspaceReviewSession,
 	type WorkspaceDiffType,
@@ -71,7 +73,7 @@ export interface BrowserDecisionSession<T> {
 type CodeReviewOptions = {
 	cwd?: string;
 	defaultBranch?: string;
-	diffType?: DiffType;
+	diffType?: string;
 	prUrl?: string;
 	vcsType?: VcsSelection;
 	useLocal?: boolean;
@@ -412,7 +414,7 @@ async function createCodeReviewBrowserSession(
 				parsed: { base: options.defaultBranch, diffType: options.diffType },
 				isPRMode: true,
 				isWorkspace: false,
-				resolvedDefaultDiffType: resolveDefaultDiffType(loadConfig()),
+				resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(loadConfig()),
 			});
 			if (openState.error) throw new Error(openState.error);
 		}
@@ -593,7 +595,14 @@ async function createCodeReviewBrowserSession(
 		// resolve the effective requested base/diff type (promotion included).
 		// Programmatic callers keep the verbatim pass-through below.
 		let requestedBase = options.defaultBranch;
-		let requestedDiffType = options.diffType;
+		let requestedDiffType: DiffType | undefined;
+		if (!openStateFromFlags && options.diffType !== undefined) {
+			const reviewPolicy = managedVcs?.reviewPolicy ?? getVcsReviewPolicy(options.vcsType);
+			if (!reviewPolicy.ownsDiffType(options.diffType)) {
+				throw new Error(`Diff type ${options.diffType} is not available for this VCS provider.`);
+			}
+			requestedDiffType = options.diffType;
+		}
 		if (openStateFromFlags) {
 			const { resolveReviewOpenState } = await import("./generated/review-open-state.ts");
 			if (managedVcs || forcedVcs) {
@@ -618,8 +627,10 @@ async function createCodeReviewBrowserSession(
 					parsed: { base: requestedBase, diffType: requestedDiffType },
 					isPRMode: false,
 					isWorkspace: false,
-					providerId,
-					resolvedDefaultDiffType: resolveDefaultDiffType(config),
+					provider: managedVcs
+						? { resolve: managedVcs.reviewPolicy.resolveOpenState }
+						: undefined,
+					resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config, providerId),
 					baseResolves,
 				});
 				if (openState.error) throw new Error(openState.error);
@@ -631,7 +642,7 @@ async function createCodeReviewBrowserSession(
 					parsed: { base: requestedBase, diffType: requestedDiffType },
 					isPRMode: false,
 					isWorkspace: true,
-					resolvedDefaultDiffType: resolveDefaultDiffType(config),
+					resolvedDefaultDiffType: resolveConfiguredVcsReviewDefault(config),
 				});
 				if (openState.error) throw new Error(openState.error);
 			}
@@ -642,7 +653,7 @@ async function createCodeReviewBrowserSession(
 				vcsType: options.vcsType,
 				requestedDiffType,
 				requestedBase,
-				configuredDiffType: resolveDefaultDiffType(config),
+				configuredDiffType: resolveConfiguredVcsReviewDefault(config, (managedVcs?.id as VcsSelection | undefined) ?? options.vcsType),
 				hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
 			});
 			gitCtx = result.gitContext;
@@ -660,8 +671,8 @@ async function createCodeReviewBrowserSession(
 			initialBaseExplicit = openStateFromFlags && requestedBase !== undefined;
 		} else {
 			workspace = await buildLocalWorkspaceReview(cwd, {
-				requestedDiffType: options.diffType,
-				configuredDiffType: resolveDefaultDiffType(config),
+				requestedDiffType,
+				configuredDiffType: resolveConfiguredVcsReviewDefault(config),
 				hideWhitespace: config.diffOptions?.hideWhitespace ?? false,
 			});
 			if (workspace.repos.length === 0) {

apps/pi-extension/review-args-parity.test.ts

@@ -28,11 +28,12 @@ describe("vendored review-args parity", () => {
     // Guards the vendor.sh entry for review-open-state: without it Pi's
     // review command would crash on import instead of validating.
     const { resolveReviewOpenState } = await import("./generated/review-open-state.ts");
+    const { jjReviewPolicy } = await import("./generated/jj-review-policy.ts");
     const state = resolveReviewOpenState({
       parsed: { base: "main" },
       isPRMode: false,
       isWorkspace: false,
-      providerId: "jj",
+      provider: { resolve: jjReviewPolicy.resolveOpenState },
       resolvedDefaultDiffType: "since-base",
     });
     expect(state.error).toContain("--base is not supported in jj sessions");

apps/pi-extension/server.ts

@@ -32,9 +32,11 @@ export {
 	detectVcs,
 	getGitContext,
 	getVcsContext,
+	getVcsReviewPolicy,
 	getVcsDiffFingerprint,
 	getVcsFileContentsForDiff,
 	prepareLocalReviewDiff,
+	resolveConfiguredVcsReviewDefault,
 	resolveInitialDiffType,
 	resolveVcsCwd,
 	reviewRuntime,

apps/pi-extension/server/serverReview.ts

@@ -3207,10 +3207,11 @@ export async function startReviewServer(options: {
 			}
 		} else if (url.pathname === "/api/config" && req.method === "POST") {
 			try {
-				const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record<string, unknown>; theme?: Record<string, unknown>; favicon?: FaviconStyle; reviewAnalysis?: Record<string, unknown>; conventionalComments?: boolean };
+				const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record<string, unknown>; reviewDefaults?: Record<string, { defaultDiffType?: string }>; theme?: Record<string, unknown>; favicon?: FaviconStyle; reviewAnalysis?: Record<string, unknown>; conventionalComments?: boolean };
 				const toSave: Record<string, unknown> = {};
 				if (body.displayName !== undefined) toSave.displayName = body.displayName;
 				if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions;
+				if (body.reviewDefaults !== undefined) toSave.reviewDefaults = body.reviewDefaults;
 				if (body.theme !== undefined) toSave.theme = body.theme;
 				if (isFaviconStyle(body.favicon)) toSave.favicon = body.favicon;
 				if (body.reviewAnalysis !== undefined) {

apps/pi-extension/server/vcs.ts

@@ -20,6 +20,10 @@ import {
 import {
 	type ReviewGitButlerRuntime,
 } from "../generated/gitbutler-core.ts";
+import type { PlannotatorConfig } from "../generated/config.ts";
+import { gitReviewPolicy } from "../generated/git-review-policy.ts";
+import { gitButlerReviewPolicy } from "../generated/gitbutler-review-policy.ts";
+import { jjReviewPolicy } from "../generated/jj-review-policy.ts";
 import {
 	type VcsSelection,
 	createGitButlerProvider,
@@ -188,12 +192,14 @@ export const gitButlerRuntime: ReviewGitButlerRuntime = {
 };
 
 const api = createVcsApi([
-	createJjProvider(jjRuntime, reviewRuntime),
-	createGitButlerProvider(gitButlerRuntime),
-	createGitProvider(reviewRuntime),
-]);
+	createJjProvider(jjRuntime, reviewRuntime, jjReviewPolicy),
+	createGitButlerProvider(gitButlerRuntime, gitButlerReviewPolicy),
+	createGitProvider(reviewRuntime, gitReviewPolicy),
+], "git");
 
 export const {
+	getReviewPolicy: getVcsReviewPolicy,
+	resolveReviewDefault: resolveVcsReviewDefault,
 	detectVcs,
 	detectManagedVcs,
 	vcsOwnsDiffType,
@@ -211,6 +217,17 @@ export const {
 	materializeVcsSnapshot,
 } = api;
 
+export function resolveConfiguredVcsReviewDefault(
+	config: PlannotatorConfig | undefined,
+	vcsType?: VcsSelection,
+): DiffType {
+	return resolveVcsReviewDefault(
+		vcsType,
+		config?.reviewDefaults,
+		config?.diffOptions?.defaultDiffType,
+	);
+}
+
 export { resolveAvailableDiffType, resolveInitialDiffType };
 export type { VcsSelection };
 

apps/pi-extension/vendor.sh

@@ -29,7 +29,7 @@ for f in config-types storage-types workspace-status-types; do
 done
 
 # Everything else in the original flat list stays sourced from packages/shared.
-for f in prompts review-core generated-files feedback-archive cli-pagination jj-core gitbutler-core vcs-core review-args review-open-state draft annotate-history pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common markdown-extensions resolve-file doc-resolve file-browser-watch-core annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args annotate-target at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff call-flow-types call-flow-languages call-flow-pack-locks call-flow-install-lock call-flow call-flow-install single-flight source-save-node review-profiles guide-store guide-instructions-store commit-avatars commit-history port-range annotate-client-lease annotate-decision archive-mode tailscale live-proxy-core live-probe live-proxy-node; do
+for f in prompts review-core generated-files feedback-archive cli-pagination jj-core gitbutler-core vcs-core vcs-review-policy git-review-policy jj-review-policy gitbutler-review-policy p4-review-policy review-args review-open-state draft annotate-history pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common markdown-extensions resolve-file doc-resolve file-browser-watch-core annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args annotate-target at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff call-flow-types call-flow-languages call-flow-pack-locks call-flow-install-lock call-flow call-flow-install single-flight source-save-node review-profiles guide-store guide-instructions-store commit-avatars commit-history port-range annotate-client-lease annotate-decision archive-mode tailscale live-proxy-core live-probe live-proxy-node; do
   src="../../packages/shared/$f.ts"
   # Shared modules that import browser-safe siblings from @plannotator/core
   # (e.g. guide-store → core/guide-format): generated/ is flat and vendors the