Skip to content

feat: enable local live diff in Review panel without a PR #341

Description

@j-marwitz-code

Problem

The Review panel (frontend/src/components/panels/diff/DiffPanel.tsx) was fully gated on an existing PR — an early return blocked the entire panel whenever session.gitStatus.prUrl was empty, even though the local diff mode (CombinedDiffView, live working-tree diff) never needed a PR at all. On top of that, SessionView.tsx independently gated the Review tab itself (disabled state, panel selection, default-panel picking, layout sanitization) on the same prUrl check in 7 places, so even fixing the panel alone wouldn't have made it reachable.

Proposed changes

1. SessionView.tsx — removed all hasReviewPr gating (canSelectPanel, pickDefaultPanel, getPanelTabPresentation, handleGroupPanelSelect, handlePanelSelect, handleFocusGroup, handleCommitClick; deleted sanitizeReviewActivePanels entirely). The Review tab is now always selectable/focusable/defaultable — PR-gating only applies inside the panel to the GitHub sub-mode.

2. DiffPanel.tsx — removed the early return. The effective mode now falls back to local when there's no PR, via a one-directional self-correcting effect that only touches local React state (never persists to the stored default preference), so the mode doesn't snap back to GitHub automatically once a PR appears mid-session — it only enables the button. Header shows "Local changes" instead of the PR title when there's no PR. The GitHub toggle is disabled with a tooltip ("Open a PR to enable") following the existing disabled-button pattern used elsewhere (PanelTabBar.tsx).

3. CombinedDiffView.tsx (bugfix found during manual testing) — mountedRef was permanently stuck false under React 18 StrictMode's dev-mode mount→cleanup→remount simulation, because the effect setup never reset it to true (only cleanup set it false). Since diff panels fully unmount whenever inactive, every fresh open of the Review tab hit this, silently discarding the getExecutions response before it could set the default selection — so the local diff never rendered. This is a pre-existing bug, only surfaced now because local mode is the default when there's no PR (previously most Review-panel opens landed on the GitHub browser view by default). Fixed by setting mountedRef.current = true in the effect setup.

Checks

  • pnpm typecheck — pass
  • pnpm lint — pass, no new warnings
  • Manual test in a dev instance: Review tab selectable without a PR, local live diff renders correctly (main-repo session, 0 commits ahead + uncommitted changes), GitHub toggle disabled with tooltip as expected
Full diff (2 commits, 3 files, +56/-90)
diff --git a/frontend/src/components/SessionView.tsx b/frontend/src/components/SessionView.tsx
index 5c5d76f..f9c594b 100644
--- a/frontend/src/components/SessionView.tsx
+++ b/frontend/src/components/SessionView.tsx
@@ -58,40 +58,17 @@ import { Kbd } from './ui/Kbd';
 import { useErrorStore } from '../stores/errorStore';
 import ProjectSettings from './ProjectSettings';
 
-const REVIEW_UNAVAILABLE_REASON = 'Open a PR for this branch to review changes';
-
-function canSelectPanel(panel: ToolPanel | null | undefined, hasReviewPr: boolean): panel is ToolPanel {
-  return !!panel && (panel.type !== 'diff' || hasReviewPr);
+function canSelectPanel(panel: ToolPanel | null | undefined): panel is ToolPanel {
+  return !!panel;
 }
 
-function pickDefaultPanel(panelList: ToolPanel[], hasReviewPr: boolean): ToolPanel | undefined {
-  return (hasReviewPr ? panelList.find(p => p.type === 'diff') : undefined)
+function pickDefaultPanel(panelList: ToolPanel[]): ToolPanel | undefined {
+  return panelList.find(p => p.type === 'diff')
     || panelList.find(p => p.type === 'explorer')
     || panelList.find(p => p.type !== 'diff')
     || panelList[0];
 }
 
-function sanitizeReviewActivePanels(
-  node: SessionPanelLayout['root'],
-  panelById: Map<string, ToolPanel>,
-  hasReviewPr: boolean
-): SessionPanelLayout['root'] {
-  if (hasReviewPr) return node;
-
-  if (node.type === 'group') {
-    const activePanel = node.activePanelId ? panelById.get(node.activePanelId) : undefined;
-    if (activePanel?.type !== 'diff') return node;
-
-    const fallback = node.panelIds
-      .map(id => panelById.get(id))
-      .find((panel): panel is ToolPanel => !!panel && panel.type !== 'diff');
-
-    return { ...node, activePanelId: fallback?.id ?? null };
-  }
-
-  return { ...node, children: node.children.map(child => sanitizeReviewActivePanels(child, panelById, hasReviewPr)) };
-}
-
 export const SessionView = memo(() => {
   const { activeView, activeProjectId } = useNavigationStore();
   const [projectData, setProjectData] = useState<Project | null>(null);
@@ -250,17 +227,16 @@ export const SessionView = memo(() => {
       // Always reload panels from database when switching sessions
       panelApi.loadPanelsForSession(sid).then(async loadedPanels => {
         devLog.debug('[SessionView] Loaded panels:', loadedPanels);
-        const hasReviewPr = !!activeSession.gitStatus?.prUrl;
         const inFlight = (usePanelStore.getState().panels[sid] || []).filter(
           p => !preLoadIds.has(p.id) && !loadedPanels.some(lp => lp.id === p.id)
         );
         setPanels(sid, inFlight.length > 0 ? [...loadedPanels, ...inFlight] : loadedPanels);
 
-        // Pick default active: Review is only selectable once a PR is known.
-        const fallback = pickDefaultPanel(loadedPanels, hasReviewPr);
+        // Pick default active panel.
+        const fallback = pickDefaultPanel(loadedPanels);
 
         const activePanelResult = await panelApi.getActivePanel(sid);
-        const effectiveActivePanel = canSelectPanel(activePanelResult, hasReviewPr)
+        const effectiveActivePanel = canSelectPanel(activePanelResult)
           ? activePanelResult
           : fallback;
         const fallbackActiveId = effectiveActivePanel?.id ?? null;
@@ -307,22 +283,16 @@ export const SessionView = memo(() => {
             fallbackActiveId,
           );
           const { layout } = reconcileLayout(base, liveIdsNow);
-          const panelById = new Map(nowPanels.map(panel => [panel.id, panel]));
-          const safeRoot = sanitizeReviewActivePanels(layout.root, panelById, hasReviewPr);
-          const safeLayout = safeRoot === layout.root ? layout : { ...layout, root: safeRoot };
-          setLayoutInStore(sid, safeLayout);
-          setFocusedGroupInStore(sid, safeLayout.focusedGroupId ?? primaryGroup(safeLayout.root).id);
+          setLayoutInStore(sid, layout);
+          setFocusedGroupInStore(sid, layout.focusedGroupId ?? primaryGroup(layout.root).id);
         } catch (err) {
           console.warn('[SessionView] Failed to load layout, creating default:', err);
           const layout = createSingleGroupLayout(
             sortedLive.map(p => p.id),
             fallbackActiveId,
           );
-          const panelById = new Map(loadedPanels.map(panel => [panel.id, panel]));
-          const safeRoot = sanitizeReviewActivePanels(layout.root, panelById, hasReviewPr);
-          const safeLayout = safeRoot === layout.root ? layout : { ...layout, root: safeRoot };
-          setLayoutInStore(sid, safeLayout);
-          setFocusedGroupInStore(sid, safeLayout.focusedGroupId ?? primaryGroup(safeLayout.root).id);
+          setLayoutInStore(sid, layout);
+          setFocusedGroupInStore(sid, layout.focusedGroupId ?? primaryGroup(layout.root).id);
         }
       });
     }
@@ -331,7 +301,7 @@ export const SessionView = memo(() => {
     return () => {
       flushLayoutPersist();
     };
-  }, [activeSession?.id, activeSession?.gitStatus?.prUrl, setPanels, setActivePanelInStore, setLayoutInStore, setFocusedGroupInStore, flushLayoutPersist]);
+  }, [activeSession?.id, setPanels, setActivePanelInStore, setLayoutInStore, setFocusedGroupInStore, flushLayoutPersist]);
   
   // Listen for panel updates from the backend
   useEffect(() => {
@@ -495,13 +465,8 @@ export const SessionView = memo(() => {
 
   const getPanelTabPresentation = useCallback<PanelTabPresentationResolver>((panel) => {
     if (panel.type !== 'diff') return undefined;
-    const hasReviewPr = !!activeSession?.gitStatus?.prUrl;
-    return {
-      title: 'Review',
-      disabled: !hasReviewPr,
-      disabledReason: hasReviewPr ? undefined : REVIEW_UNAVAILABLE_REASON,
-    };
-  }, [activeSession?.gitStatus?.prUrl]);
+    return { title: 'Review' };
+  }, []);
 
   // --- Drag & drop state ---
   const [draggedPanelId, setDraggedPanelId] = useState<string | null>(null);
@@ -553,7 +518,6 @@ export const SessionView = memo(() => {
   const handleGroupPanelSelect = useCallback(
     (groupId: string, panel: ToolPanel) => {
       if (!activeSession) return;
-      if (panel.type === 'diff' && !activeSession.gitStatus?.prUrl) return;
       const sid = activeSession.id;
       const currentLayout = usePanelStore.getState().layouts[sid];
       if (!currentLayout) return;
@@ -584,7 +548,6 @@ export const SessionView = memo(() => {
   const handlePanelSelect = useCallback(
     async (panel: ToolPanel) => {
       if (!activeSession) return;
-      if (panel.type === 'diff' && !activeSession.gitStatus?.prUrl) return;
 
       // Add to history when panel is selected
       addToHistory(activeSession.id, panel.id);
@@ -608,7 +571,6 @@ export const SessionView = memo(() => {
   const handleCommitClick = useCallback(
     async (commitHash: string) => {
       if (!activeSession || sessionPanels.length === 0) return;
-      if (!activeSession.gitStatus?.prUrl) return;
       const diffPanel = sessionPanels.find(p => p.type === 'diff');
       if (!diffPanel) return;
       // Store pending hash before dispatching — if the diff panel is not
@@ -1033,10 +995,6 @@ export const SessionView = memo(() => {
     const currentLayout = usePanelStore.getState().layouts[sid];
     if (currentLayout) {
       const group = findGroup(currentLayout.root, groupId);
-      const panel = group?.activePanelId
-        ? usePanelStore.getState().panels[sid]?.find(candidate => candidate.id === group.activePanelId)
-        : undefined;
-      if (panel?.type === 'diff' && !activeSession.gitStatus?.prUrl) return;
       if (group?.activePanelId) {
         setActivePanelInStore(sid, group.activePanelId);
         panelApi.setActivePanel(sid, group.activePanelId).catch(() => {});
diff --git a/frontend/src/components/panels/diff/CombinedDiffView.tsx b/frontend/src/components/panels/diff/CombinedDiffView.tsx
index ac1b9e0..624cc87 100644
--- a/frontend/src/components/panels/diff/CombinedDiffView.tsx
+++ b/frontend/src/components/panels/diff/CombinedDiffView.tsx
@@ -117,6 +117,7 @@ const CombinedDiffView = memo(forwardRef<CombinedDiffViewHandle, CombinedDiffVie
   const showDiffSkeleton = (diffLoading || commitDiffLoading) && combinedDiff === null;
 
   useEffect(() => {
+    mountedRef.current = true;
     return () => { mountedRef.current = false; };
   }, []);
 
diff --git a/frontend/src/components/panels/diff/DiffPanel.tsx b/frontend/src/components/panels/diff/DiffPanel.tsx
index 2cf82eb..6674f92 100644
--- a/frontend/src/components/panels/diff/DiffPanel.tsx
+++ b/frontend/src/components/panels/diff/DiffPanel.tsx
@@ -6,6 +6,7 @@ import type { GitStatus } from '../../../types/session';
 import { AlertCircle, GitBranch, Globe } from 'lucide-react';
 import { useSession } from '../../../contexts/SessionContext';
 import { cn } from '../../../utils/cn';
+import { Tooltip } from '../../ui/Tooltip';
 import BrowserSurface from '../browser/BrowserSurface';
 import {
   consumeLocalReviewModeRequest,
@@ -96,6 +97,15 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
     setReviewDefaultMode(mode);
   }, []);
 
+  // Fall back to local mode when there's no PR to review against. This only
+  // adjusts the effective React state, not the stored preference, so a PR
+  // appearing later restores the user's chosen GitHub default.
+  useEffect(() => {
+    if (!reviewUrl && reviewMode === 'github') {
+      setReviewModeState('local');
+    }
+  }, [reviewUrl, reviewMode]);
+
   // Listen for file change events from other panels
   useEffect(() => {
     const handlePanelEvent = (event: CustomEvent) => {
@@ -190,22 +200,6 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
   // eslint-disable-next-line react-hooks/exhaustive-deps -- panel.state/diffState intentionally excluded: they are written inside this effect via IPC and must not re-trigger it
   }, [isActive, isStale, panel.id, sessionId, reviewMode]);
 
-  if (!reviewUrl) {
-    return (
-      <div className="diff-panel h-full flex flex-col bg-bg-primary">
-        <div className="flex-1 flex items-center justify-center p-8 text-text-secondary">
-          <div className="max-w-sm text-center space-y-2">
-            <GitBranch className="w-8 h-8 mx-auto text-text-muted" />
-            <h2 className="text-sm font-medium text-text-primary">Review unavailable</h2>
-            <p className="text-xs text-text-tertiary">
-              Open a PR for this branch to review changes.
-            </p>
-          </div>
-        </div>
-      </div>
-    );
-  }
-
   const prLabel = session?.gitStatus?.prNumber ? `#${session.gitStatus.prNumber}` : 'Pull Request';
 
   return (
@@ -214,27 +208,40 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
         <div className="flex items-center gap-2 min-w-0">
           <GitBranch className="w-3.5 h-3.5 text-text-tertiary flex-shrink-0" />
           <span className="text-xs font-medium text-text-secondary truncate">Review</span>
-          <span className="text-xs text-text-muted truncate">{prLabel}</span>
-          {session?.gitStatus?.prTitle && (
-            <span className="text-xs text-text-tertiary truncate">{session.gitStatus.prTitle}</span>
+          {reviewUrl ? (
+            <>
+              <span className="text-xs text-text-muted truncate">{prLabel}</span>
+              {session?.gitStatus?.prTitle && (
+                <span className="text-xs text-text-tertiary truncate">{session.gitStatus.prTitle}</span>
+              )}
+            </>
+          ) : (
+            <span className="text-xs text-text-muted truncate">Local changes</span>
           )}
         </div>
 
         <div className="inline-flex items-center rounded border border-border-primary bg-bg-primary p-0.5 flex-shrink-0">
-          <button
-            type="button"
-            onClick={() => handleReviewModeChange('github')}
-            className={cn(
-              "inline-flex items-center gap-1 px-2 py-0.5 rounded text-xs transition-colors",
-              reviewMode === 'github'
-                ? "bg-interactive text-text-on-interactive"
-                : "text-text-secondary hover:bg-surface-hover"
-            )}
-            aria-pressed={reviewMode === 'github'}
-          >
-            <Globe className="w-3 h-3" />
-            GitHub
-          </button>
+          <Tooltip content={!reviewUrl ? 'Open a PR to enable' : undefined} side="bottom">
+            <button
+              type="button"
+              onClick={() => reviewUrl && handleReviewModeChange('github')}
+              disabled={!reviewUrl}
+              aria-disabled={!reviewUrl}
+              className={cn(
+                "inline-flex items-center gap-1 px-2 py-0.5 rounded text-xs transition-colors",
+                !reviewUrl
+                  ? "text-text-muted cursor-not-allowed opacity-50"
+                  : reviewMode === 'github'
+                  ? "bg-interactive text-text-on-interactive"
+                  : "text-text-secondary hover:bg-surface-hover"
+              )}
+              aria-pressed={reviewMode === 'github'}
+              title={!reviewUrl ? 'Open a PR to enable' : undefined}
+            >
+              <Globe className="w-3 h-3" />
+              GitHub
+            </button>
+          </Tooltip>
           <button
             type="button"
             onClick={() => handleReviewModeChange('local')}
@@ -264,7 +271,7 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
 
       {/* Main diff view */}
       <div className="flex-1 overflow-hidden">
-        {reviewMode === 'github' ? (
+        {reviewMode === 'github' && reviewUrl ? (
           <BrowserSurface
             panelId={panel.id}
             sessionId={sessionId}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions