fix: make review diff refresh immediate and non-interactive

- Manual refresh now bumps refreshTrigger immediately (no blocking fetch)
- Move origin/* git fetch into diff/tree load with GIT_TERMINAL_PROMPT=0
- Share origin fetch across file tree + diff via a keyed promise ref

Author: ammar-agent

src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx

@@ -143,6 +143,53 @@ const reviewPanelCache = new LRUCache<string, ReviewPanelCacheValue>({
   sizeCalculation: (value) => estimateJsonSizeBytes(value),
 });
 
+function getOriginBranchForFetch(diffBase: string): string | null {
+  const trimmed = diffBase.trim();
+  if (!trimmed.startsWith("origin/")) return null;
+
+  const branch = trimmed.slice("origin/".length);
+
+  // Avoid shell injection; diffBase is user-controlled.
+  if (!/^[0-9A-Za-z._/-]+$/.test(branch)) return null;
+
+  return branch;
+}
+
+interface OriginFetchState {
+  key: string;
+  promise: Promise<void>;
+}
+
+async function ensureOriginFetched(params: {
+  api: APIClient;
+  workspaceId: string;
+  diffBase: string;
+  refreshToken: number;
+  originFetchRef: React.MutableRefObject<OriginFetchState | null>;
+}): Promise<void> {
+  const originBranch = getOriginBranchForFetch(params.diffBase);
+  if (!originBranch) return;
+
+  const key = [params.workspaceId, params.diffBase, String(params.refreshToken)].join("\u0000");
+  const existing = params.originFetchRef.current;
+  if (existing?.key === key) {
+    await existing.promise;
+    return;
+  }
+
+  // Ensure manual refresh doesn't hang on credential prompts.
+  const promise = params.api.workspace
+    .executeBash({
+      workspaceId: params.workspaceId,
+      script: `GIT_TERMINAL_PROMPT=0 git fetch origin ${originBranch} --quiet || true`,
+      options: { timeout_secs: 30 },
+    })
+    .then(() => undefined)
+    .catch(() => undefined);
+
+  params.originFetchRef.current = { key, promise };
+  await promise;
+}
 function makeReviewPanelCacheKey(params: {
   workspaceId: string;
   workspacePath: string;
@@ -186,6 +233,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
   isCreating = false,
   onStatsChange,
 }) => {
+  const originFetchRef = useRef<OriginFetchState | null>(null);
   const { api } = useAPI();
   const panelRef = useRef<HTMLDivElement>(null);
   const scrollContainerRef = useRef<HTMLDivElement>(null);
@@ -334,6 +382,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
     const loadFileTree = async () => {
       setIsLoadingTree(true);
       try {
+        await ensureOriginFetched({
+          api,
+          workspaceId,
+          diffBase: filters.diffBase,
+          refreshToken: refreshTrigger,
+          originFetchRef,
+        });
+        if (cancelled) return;
+
         const tree = await executeWorkspaceBashAndCache({
           api,
           workspaceId,
@@ -445,6 +502,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
 
     const loadDiff = async () => {
       try {
+        await ensureOriginFetched({
+          api,
+          workspaceId,
+          diffBase: filters.diffBase,
+          refreshToken: refreshTrigger,
+          originFetchRef,
+        });
+        if (cancelled) return;
+
         // Git-level filters (affect what data is fetched):
         // - diffBase: what to diff against
         // - includeUncommitted: include working directory changes

src/browser/hooks/useReviewRefreshController.ts

@@ -7,22 +7,6 @@ import { usePersistedState } from "@/browser/hooks/usePersistedState";
 /** Debounce delay for auto-refresh after tool completion */
 const TOOL_REFRESH_DEBOUNCE_MS = 3000;
 
-/**
- * Extract branch name from an "origin/..." diff base for git fetch.
- * Returns null if not an origin ref or if branch name is unsafe for shell.
- */
-function getOriginBranchForFetch(diffBase: string): string | null {
-  const trimmed = diffBase.trim();
-  if (!trimmed.startsWith("origin/")) return null;
-
-  const branch = trimmed.slice("origin/".length);
-
-  // Avoid shell injection; diffBase is user-controlled.
-  if (!/^[0-9A-Za-z._/-]+$/.test(branch)) return null;
-
-  return branch;
-}
-
 export interface UseReviewRefreshControllerOptions {
   workspaceId: string;
   api: APIClient | null;
@@ -53,7 +37,6 @@ export interface ReviewRefreshController {
  *
  * Delegates debouncing, visibility/focus handling, and in-flight guards to RefreshController.
  * Keeps ReviewPanel-specific logic:
- * - Origin branch fetch before refresh
  * - Scroll position preservation
  * - User interaction pause state
  */
@@ -63,9 +46,6 @@ export function useReviewRefreshController(
   const { workspaceId, api, isCreating, scrollContainerRef } = options;
 
   // Refs for values that executeRefresh needs at call time (avoid stale closures)
-  const diffBaseRef = useRef(options.diffBase);
-  diffBaseRef.current = options.diffBase;
-
   const onRefreshRef = useRef(options.onRefresh);
   onRefreshRef.current = options.onRefresh;
 
@@ -89,35 +69,22 @@ export function useReviewRefreshController(
     const ctrl = new RefreshController({
       debounceMs: TOOL_REFRESH_DEBOUNCE_MS,
       isPaused: () => isInteractingRef.current,
-      onRefresh: async () => {
-        if (!api || isCreating) return;
+      onRefresh: () => {
+        if (isCreating) return;
 
         // Save scroll position before refresh
         savedScrollTopRef.current = scrollContainerRef.current?.scrollTop ?? null;
 
-        const originBranch = getOriginBranchForFetch(diffBaseRef.current);
-        if (originBranch) {
-          try {
-            await api.workspace.executeBash({
-              workspaceId,
-              script: `git fetch origin ${originBranch} --quiet || true`,
-              options: { timeout_secs: 30 },
-            });
-          } catch (err) {
-            console.debug("ReviewPanel origin fetch failed", err);
-          }
-        }
-
         onRefreshRef.current();
         onGitStatusRefreshRef.current?.();
       },
       onRefreshComplete: setLastRefreshInfo,
     });
     ctrl.bindListeners();
     return ctrl;
-    // workspaceId/api/isCreating changes require new controller with updated closure
-    // Note: options.onRefresh is accessed via ref to avoid recreating controller on every render
-  }, [workspaceId, api, isCreating, scrollContainerRef, setLastRefreshInfo]);
+    // isCreating/scrollContainerRef/setLastRefreshInfo changes require a new controller.
+    // Note: options.onRefresh is accessed via ref to avoid recreating controller on every render.
+  }, [isCreating, scrollContainerRef, setLastRefreshInfo]);
 
   // Cleanup on unmount or when controller changes
   useEffect(() => {