From 559bdd33bf975fd9d007368a4d9f9b77e1296f5b Mon Sep 17 00:00:00 2001 From: parsakhaz Date: Tue, 21 Jul 2026 17:15:19 -0700 Subject: [PATCH 1/2] Enable local Review without a PR, closes #341 --- frontend/src/components/SessionView.tsx | 70 +++------------ .../panels/diff/CombinedDiffView.tsx | 1 + .../src/components/panels/diff/DiffPanel.tsx | 89 +++++++++++-------- .../panels/diff/reviewModePreference.test.ts | 17 ++++ .../panels/diff/reviewModePreference.ts | 4 + 5 files changed, 86 insertions(+), 95 deletions(-) create mode 100644 frontend/src/components/panels/diff/reviewModePreference.test.ts diff --git a/frontend/src/components/SessionView.tsx b/frontend/src/components/SessionView.tsx index 5c5d76f0..f9c594b7 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, - 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(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((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(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 ac1b9e06..624cc87f 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 { + 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 2cf82ebb..7b3adfdf 100644 --- a/frontend/src/components/panels/diff/DiffPanel.tsx +++ b/frontend/src/components/panels/diff/DiffPanel.tsx @@ -6,9 +6,11 @@ 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, + getEffectiveReviewMode, getReviewDefaultMode, setReviewDefaultMode, subscribeReviewDefaultMode, @@ -76,6 +78,7 @@ export const DiffPanel: React.FC = ({ const lastGitFingerprintRef = useRef(null); const wasActiveRef = useRef(isActive); const reviewUrl = useMemo(() => buildGithubReviewUrl(session?.gitStatus?.prUrl), [session?.gitStatus?.prUrl]); + const effectiveReviewMode = getEffectiveReviewMode(reviewMode, !!reviewUrl); useEffect(() => subscribeReviewDefaultMode(setReviewModeState), []); @@ -96,6 +99,12 @@ export const DiffPanel: React.FC = ({ setReviewDefaultMode(mode); }, []); + useEffect(() => { + if (effectiveReviewMode !== reviewMode) { + setReviewModeState(effectiveReviewMode); + } + }, [effectiveReviewMode, reviewMode]); + // Listen for file change events from other panels useEffect(() => { const handlePanelEvent = (event: CustomEvent) => { @@ -154,7 +163,7 @@ export const DiffPanel: React.FC = ({ const becameActive = isActive && !wasActiveRef.current; wasActiveRef.current = isActive; - if (becameActive && isStale && reviewMode === 'local') { + if (becameActive && isStale && effectiveReviewMode === 'local') { setIsStale(false); combinedDiffRef.current?.refresh(); @@ -188,25 +197,30 @@ export const DiffPanel: React.FC = ({ return () => clearTimeout(timer); } // 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 ( -
-
-
- -

Review unavailable

-

- Open a PR for this branch to review changes. -

-
-
-
- ); - } + }, [effectiveReviewMode, isActive, isStale, panel.id, sessionId]); const prLabel = session?.gitStatus?.prNumber ? `#${session.gitStatus.prNumber}` : 'Pull Request'; + const githubModeButton = ( + + ); return (
@@ -214,37 +228,34 @@ export const DiffPanel: React.FC = ({
Review - {prLabel} - {session?.gitStatus?.prTitle && ( - {session.gitStatus.prTitle} + {reviewUrl ? ( + <> + {prLabel} + {session?.gitStatus?.prTitle && ( + {session.gitStatus.prTitle} + )} + + ) : ( + Local changes )}
- + {reviewUrl ? githubModeButton : ( + + {githubModeButton} + + )}
{/* Stale indicator bar */} - {reviewMode === 'local' && isStale && !isActive && ( + {effectiveReviewMode === 'local' && isStale && !isActive && (
@@ -264,7 +275,7 @@ export const DiffPanel: React.FC = ({ {/* Main diff view */}
- {reviewMode === 'github' ? ( + {effectiveReviewMode === 'github' && reviewUrl ? ( { + it('falls back from GitHub to local mode when no PR URL is available', () => { + expect(getEffectiveReviewMode('github', false)).toBe('local'); + }); + + it('keeps GitHub mode when a PR URL is available', () => { + expect(getEffectiveReviewMode('github', true)).toBe('github'); + }); + + it('keeps explicit local mode regardless of PR URL availability', () => { + expect(getEffectiveReviewMode('local', false)).toBe('local'); + expect(getEffectiveReviewMode('local', true)).toBe('local'); + }); +}); diff --git a/frontend/src/components/panels/diff/reviewModePreference.ts b/frontend/src/components/panels/diff/reviewModePreference.ts index 446d3fff..eb9aaf99 100644 --- a/frontend/src/components/panels/diff/reviewModePreference.ts +++ b/frontend/src/components/panels/diff/reviewModePreference.ts @@ -15,6 +15,10 @@ export function getReviewDefaultMode(): ReviewMode { return isReviewMode(stored) ? stored : 'github'; } +export function getEffectiveReviewMode(mode: ReviewMode, hasReviewUrl: boolean): ReviewMode { + return mode === 'github' && !hasReviewUrl ? 'local' : mode; +} + export function setReviewDefaultMode(mode: ReviewMode): void { window.localStorage.setItem(REVIEW_MODE_STORAGE_KEY, mode); window.dispatchEvent(new CustomEvent(REVIEW_MODE_CHANGED_EVENT, { detail: { mode } })); From c72ca71677054fd025860e912d9f180ca847c0bb Mon Sep 17 00:00:00 2001 From: parsakhaz Date: Tue, 21 Jul 2026 17:35:23 -0700 Subject: [PATCH 2/2] Preserve GitHub review preference while PR URL loads --- frontend/src/components/panels/diff/DiffPanel.tsx | 6 ------ .../components/panels/diff/reviewModePreference.test.ts | 7 +++++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/panels/diff/DiffPanel.tsx b/frontend/src/components/panels/diff/DiffPanel.tsx index 7b3adfdf..22c7649d 100644 --- a/frontend/src/components/panels/diff/DiffPanel.tsx +++ b/frontend/src/components/panels/diff/DiffPanel.tsx @@ -99,12 +99,6 @@ export const DiffPanel: React.FC = ({ setReviewDefaultMode(mode); }, []); - useEffect(() => { - if (effectiveReviewMode !== reviewMode) { - setReviewModeState(effectiveReviewMode); - } - }, [effectiveReviewMode, reviewMode]); - // Listen for file change events from other panels useEffect(() => { const handlePanelEvent = (event: CustomEvent) => { diff --git a/frontend/src/components/panels/diff/reviewModePreference.test.ts b/frontend/src/components/panels/diff/reviewModePreference.test.ts index ba92e321..acae3f38 100644 --- a/frontend/src/components/panels/diff/reviewModePreference.test.ts +++ b/frontend/src/components/panels/diff/reviewModePreference.test.ts @@ -10,6 +10,13 @@ describe('getEffectiveReviewMode', () => { expect(getEffectiveReviewMode('github', true)).toBe('github'); }); + it('restores GitHub mode when a PR URL becomes available later', () => { + const savedMode = 'github'; + + expect(getEffectiveReviewMode(savedMode, false)).toBe('local'); + expect(getEffectiveReviewMode(savedMode, true)).toBe('github'); + }); + it('keeps explicit local mode regardless of PR URL availability', () => { expect(getEffectiveReviewMode('local', false)).toBe('local'); expect(getEffectiveReviewMode('local', true)).toBe('local');