From 087091cef8b9fbba1e64c74097ec63e70fdc1ba1 Mon Sep 17 00:00:00 2001 From: Andy <119136210+AndyMik90@users.noreply.github.com> Date: Mon, 9 Feb 2026 10:25:09 +0100 Subject: [PATCH] auto-claude: 204-fix-pr-review-ui-not-updating-without-manual-navig (#1734) * auto-claude: subtask-2-1 - Add refresh callback system to PR review store * auto-claude: subtask-2-2 - Register refresh callback in useGitHubPRs hook * auto-claude: subtask-2-3 - Ensure GitHubPRs component re-renders on PR list u Add useEffect to sync UI state when PR list updates via auto-refresh. Following the pattern from PRDetail.tsx, ensure selected PR is validated after list updates to prevent stale state when PRs are closed/merged. Co-Authored-By: Claude Sonnet 4.5 * fix: Use returned cleanup functions from IPC listeners and clear refreshCallbacks The on* IPC methods return cleanup functions but they were being ignored. Instead, the code tried to call non-existent remove* methods. Now captures the returned cleanup functions directly. Also clears refreshCallbacks in cleanupPRReviewListeners to prevent stale callbacks during HMR. Co-Authored-By: Claude Opus 4.6 * fix: Add projectId dependency, handle async callbacks, wire up listener cleanup - Add [projectId] to useEffect dependency array so state resets on project switch - Use Promise.resolve().catch() for refresh callbacks since fetchPRs is async - Export cleanupPRReviewListeners from barrel and call it in App.tsx cleanup Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Test User Co-authored-by: Claude Sonnet 4.5 --- apps/frontend/src/renderer/App.tsx | 3 +- .../components/github-prs/GitHubPRs.tsx | 16 ++++- .../github-prs/hooks/useGitHubPRs.ts | 17 +++++- .../src/renderer/stores/github/index.ts | 10 ++++ .../renderer/stores/github/pr-review-store.ts | 59 +++++++++++++++++-- 5 files changed, 98 insertions(+), 7 deletions(-) diff --git a/apps/frontend/src/renderer/App.tsx b/apps/frontend/src/renderer/App.tsx index 76b110aa..3e8eddcd 100644 --- a/apps/frontend/src/renderer/App.tsx +++ b/apps/frontend/src/renderer/App.tsx @@ -60,7 +60,7 @@ import { useTaskStore, loadTasks } from './stores/task-store'; import { useSettingsStore, loadSettings, loadProfiles, saveSettings } from './stores/settings-store'; import { useClaudeProfileStore, loadClaudeProfiles } from './stores/claude-profile-store'; import { useTerminalStore, restoreTerminalSessions } from './stores/terminal-store'; -import { initializeGitHubListeners } from './stores/github'; +import { initializeGitHubListeners, cleanupGitHubListeners } from './stores/github'; import { initDownloadProgressListener } from './stores/download-store'; import { GlobalDownloadIndicator } from './components/GlobalDownloadIndicator'; import { useIpcListeners } from './hooks/useIpc'; @@ -193,6 +193,7 @@ export function App() { return () => { cleanupDownloadListener(); + cleanupGitHubListeners(); }; }, []); diff --git a/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx b/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx index f7d997bc..9f0d186a 100644 --- a/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx +++ b/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx @@ -1,4 +1,4 @@ -import { useCallback } from "react"; +import { useCallback, useEffect } from "react"; import { GitPullRequest, RefreshCw, ExternalLink, Settings } from "lucide-react"; import { useTranslation } from "react-i18next"; import { useProjectStore } from "../../stores/project-store"; @@ -103,6 +103,20 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps) hasActiveFilters, } = usePRFiltering(prs, getReviewStateForPR); + // Sync UI state when PR list updates (e.g., after auto-refresh from review completion) + // Following pattern from PRDetail.tsx for state syncing + useEffect(() => { + // Ensure selected PR is still valid after list updates + // This prevents stale state if a PR was closed/merged while selected + if (selectedPRNumber && prs.length > 0) { + const selectedStillExists = prs.some(pr => pr.number === selectedPRNumber); + if (!selectedStillExists) { + // Selected PR was removed/closed, clear selection to prevent stale state + selectPR(null); + } + } + }, [prs, selectedPRNumber, selectPR]); + const handleRunReview = useCallback(() => { if (selectedPRNumber) { runReview(selectedPRNumber); diff --git a/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts b/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts index c018d04c..8317549e 100644 --- a/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts +++ b/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts @@ -98,6 +98,8 @@ export function useGitHubPRs( const prReviews = usePRReviewStore((state) => state.prReviews); const getPRReviewState = usePRReviewStore((state) => state.getPRReviewState); const setNewCommitsCheckAction = usePRReviewStore((state) => state.setNewCommitsCheck); + const registerRefreshCallback = usePRReviewStore((state) => state.registerRefreshCallback); + const unregisterRefreshCallback = usePRReviewStore((state) => state.unregisterRefreshCallback); // Get review state for the selected PR from the store - optimized with targeted selector // Only subscribes to changes for this specific PR, not all PRs @@ -257,7 +259,7 @@ export function useGitHubPRs( checkNewCommitsAbortRef.current.abort(); checkNewCommitsAbortRef.current = null; } - }, []); + }, [projectId]); // Cleanup abort controller on unmount to prevent memory leaks // and avoid state updates on unmounted components @@ -269,6 +271,19 @@ export function useGitHubPRs( }; }, []); + // Register refresh callback to auto-refresh PR list when reviews complete + useEffect(() => { + if (!projectId) return; + + // Register fetchPRs to be called when any PR review completes + registerRefreshCallback(fetchPRs); + + // Unregister on unmount or when dependencies change + return () => { + unregisterRefreshCallback(fetchPRs); + }; + }, [projectId, fetchPRs, registerRefreshCallback, unregisterRefreshCallback]); + // No need for local IPC listeners - they're handled globally in github-store const selectPR = useCallback( diff --git a/apps/frontend/src/renderer/stores/github/index.ts b/apps/frontend/src/renderer/stores/github/index.ts index 9c52e048..c20fa48b 100644 --- a/apps/frontend/src/renderer/stores/github/index.ts +++ b/apps/frontend/src/renderer/stores/github/index.ts @@ -23,10 +23,12 @@ export { export { usePRReviewStore, initializePRReviewListeners, + cleanupPRReviewListeners, startPRReview, startFollowupReview } from './pr-review-store'; import { initializePRReviewListeners as _initPRReviewListeners } from './pr-review-store'; +import { cleanupPRReviewListeners as _cleanupPRReviewListeners } from './pr-review-store'; // Investigation Store export { @@ -49,6 +51,14 @@ export function initializeGitHubListeners(): void { // Add other global listeners here as needed } +/** + * Cleanup all global GitHub listeners. + * Call this during app unmount or hot-reload. + */ +export function cleanupGitHubListeners(): void { + _cleanupPRReviewListeners(); +} + // Re-export types for convenience export type { PRReviewProgress, diff --git a/apps/frontend/src/renderer/stores/github/pr-review-store.ts b/apps/frontend/src/renderer/stores/github/pr-review-store.ts index 91a8bf16..5300a986 100644 --- a/apps/frontend/src/renderer/stores/github/pr-review-store.ts +++ b/apps/frontend/src/renderer/stores/github/pr-review-store.ts @@ -40,8 +40,15 @@ interface PRReviewStoreState { // Selectors getPRReviewState: (projectId: string, prNumber: number) => PRReviewState | null; getActivePRReviews: (projectId: string) => PRReviewState[]; + + // Refresh callbacks - called when reviews complete + registerRefreshCallback: (callback: () => void) => void; + unregisterRefreshCallback: (callback: () => void) => void; } +// Store for refresh callbacks outside of Zustand state (to avoid re-renders on registration) +const refreshCallbacks = new Set<() => void>(); + export const usePRReviewStore = create((set, get) => ({ // Initial state prReviews: {}, @@ -214,6 +221,15 @@ export const usePRReviewStore = create((set, get) => ({ return Object.values(prReviews).filter( review => review.projectId === projectId && review.isReviewing ); + }, + + // Refresh callbacks - called when reviews complete + registerRefreshCallback: (callback: () => void) => { + refreshCallbacks.add(callback); + }, + + unregisterRefreshCallback: (callback: () => void) => { + refreshCallbacks.delete(callback); } })); @@ -223,6 +239,7 @@ export const usePRReviewStore = create((set, get) => ({ * regardless of which component is mounted. */ let prReviewListenersInitialized = false; +let cleanupFunctions: (() => void)[] = []; export function initializePRReviewListeners(): void { if (prReviewListenersInitialized) { @@ -231,29 +248,45 @@ export function initializePRReviewListeners(): void { const store = usePRReviewStore.getState(); + // Check if GitHub PR Review API is available + if (!window.electronAPI?.github?.onPRReviewProgress) { + console.warn('[GitHub PR Store] GitHub PR Review API not available, skipping listener setup'); + return; + } + // Listen for PR review progress events - window.electronAPI.github.onPRReviewProgress( + // Each on* method returns a cleanup function — capture them for proper teardown + const cleanupProgress = window.electronAPI.github.onPRReviewProgress( (projectId: string, progress: PRReviewProgress) => { store.setPRReviewProgress(projectId, progress); } ); + cleanupFunctions.push(cleanupProgress); // Listen for PR review completion events - window.electronAPI.github.onPRReviewComplete( + const cleanupComplete = window.electronAPI.github.onPRReviewComplete( (projectId: string, result: PRReviewResult) => { store.setPRReviewResult(projectId, result); + // Trigger all registered refresh callbacks when review completes + refreshCallbacks.forEach(callback => { + Promise.resolve(callback()).catch(error => { + console.error('[PRReviewStore] Error in refresh callback:', error); + }); + }); } ); + cleanupFunctions.push(cleanupComplete); // Listen for PR review error events - window.electronAPI.github.onPRReviewError( + const cleanupError = window.electronAPI.github.onPRReviewError( (projectId: string, data: { prNumber: number; error: string }) => { store.setPRReviewError(projectId, data.prNumber, data.error); } ); + cleanupFunctions.push(cleanupError); // Listen for GitHub auth changes - clear all PR review state when account changes - window.electronAPI.github.onGitHubAuthChanged( + const cleanupAuthChanged = window.electronAPI.github.onGitHubAuthChanged( (data: { oldUsername: string | null; newUsername: string }) => { console.warn( `[PRReviewStore] GitHub auth changed from "${data.oldUsername ?? 'none'}" to "${data.newUsername}". ` + @@ -263,10 +296,28 @@ export function initializePRReviewListeners(): void { usePRReviewStore.setState({ prReviews: {} }); } ); + cleanupFunctions.push(cleanupAuthChanged); prReviewListenersInitialized = true; } +/** + * Cleanup PR review listeners. + * Call this when the app is being unmounted or during hot-reload. + */ +export function cleanupPRReviewListeners(): void { + for (const cleanup of cleanupFunctions) { + try { + cleanup(); + } catch { + // Ignore cleanup errors + } + } + cleanupFunctions = []; + refreshCallbacks.clear(); + prReviewListenersInitialized = false; +} + /** * Start a PR review and track it in the store */