diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 2ae1cc8c..c12193f3 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -385,6 +385,16 @@ function createWindow(): void { agentManager?.killAll?.()?.catch((err: unknown) => { console.warn('[main] Error killing agents on window close:', err); }); + // Clear GitLab MR polling intervals for all projects + import('./ipc-handlers/gitlab/mr-review-handlers').then(({ clearPollingForProject }) => { + const { projectStore } = require('./project-store'); + const projects = projectStore.getAllProjects(); + for (const project of projects) { + clearPollingForProject(project.id); + } + }).catch((err: unknown) => { + console.warn('[main] Error clearing GitLab polling on window close:', err); + }); mainWindow = null; }); } diff --git a/apps/desktop/src/main/ipc-handlers/gitlab/mr-review-handlers.ts b/apps/desktop/src/main/ipc-handlers/gitlab/mr-review-handlers.ts index 715bf485..6336f1c0 100644 --- a/apps/desktop/src/main/ipc-handlers/gitlab/mr-review-handlers.ts +++ b/apps/desktop/src/main/ipc-handlers/gitlab/mr-review-handlers.ts @@ -17,7 +17,7 @@ import { randomUUID } from 'crypto'; import { IPC_CHANNELS, MODEL_ID_MAP, DEFAULT_FEATURE_MODELS, DEFAULT_FEATURE_THINKING } from '../../../shared/constants'; import { getGitLabConfig, gitlabFetch, encodeProjectPath } from './utils'; import { readSettingsFile } from '../../settings-utils'; -import type { Project, AppSettings, IPCResult } from '../../../shared/types'; +import type { Project, AppSettings, IPCResult, GitLabMergeRequest } from '../../../shared/types'; import type { MRReviewResult, MRReviewProgress, @@ -25,6 +25,7 @@ import type { } from './types'; import { createContextLogger } from '../github/utils/logger'; import { withProjectOrNull } from '../github/utils/project-middleware'; +import { projectStore } from '../../project-store'; import { createIPCCommunicators } from '../github/utils/ipc-communicator'; import { MRReviewEngine, @@ -1118,6 +1119,7 @@ export function registerMRReviewHandlers( /** * Get AI review logs for an MR + * TODO: Return structured PRLogs type instead of string[] to match MRLogs component expectations */ ipcMain.handle( IPC_CHANNELS.GITLAB_MR_GET_LOGS, @@ -1163,7 +1165,7 @@ export function registerMRReviewHandlers( async (event, projectId: string, mrIid: number, intervalMs: number = 5000): Promise> => { debugLog('statusPollStart handler called', { projectId, mrIid, intervalMs }); - const result = await withProjectOrNull(projectId, async (project) => { + const result = await withProjectOrNull(projectId, async (_project) => { const pollKey = `${projectId}:${mrIid}`; // Clear existing interval if any @@ -1191,7 +1193,14 @@ export function registerMRReviewHandlers( try { // Emit status update to renderer if (callingWindow && !callingWindow.isDestroyed()) { - const config = await getGitLabConfig(project); + // Fetch current project to avoid stale config from closure + const currentProject = projectStore.getProject(projectId); + if (!currentProject) { + debugLog('Project not found during poll', { projectId }); + return; + } + + const config = await getGitLabConfig(currentProject); if (!config) return; const { token, instanceUrl } = config; @@ -1339,7 +1348,7 @@ export function registerMRReviewHandlers( projectId: string, state?: 'opened' | 'closed' | 'merged' | 'all', page: number = 2 - ): Promise> => { + ): Promise> => { debugLog('listMore handler called', { projectId, state, page }); const result = await withProjectOrNull(projectId, async (project) => { diff --git a/apps/desktop/src/main/ipc-handlers/project-handlers.ts b/apps/desktop/src/main/ipc-handlers/project-handlers.ts index e5567c17..248ba346 100644 --- a/apps/desktop/src/main/ipc-handlers/project-handlers.ts +++ b/apps/desktop/src/main/ipc-handlers/project-handlers.ts @@ -266,6 +266,9 @@ export function registerProjectHandlers( ipcMain.handle( IPC_CHANNELS.PROJECT_REMOVE, async (_, projectId: string): Promise => { + // Clear GitLab MR polling for this project to prevent memory leaks + const { clearPollingForProject } = await import('./gitlab/mr-review-handlers'); + clearPollingForProject(projectId); const success = projectStore.removeProject(projectId); return { success }; } diff --git a/apps/desktop/src/renderer/components/gitlab-issues/utils/gitlab-error-parser.ts b/apps/desktop/src/renderer/components/gitlab-issues/utils/gitlab-error-parser.ts index c698f3fa..ee81fd1f 100644 --- a/apps/desktop/src/renderer/components/gitlab-issues/utils/gitlab-error-parser.ts +++ b/apps/desktop/src/renderer/components/gitlab-issues/utils/gitlab-error-parser.ts @@ -33,6 +33,14 @@ export function parseGitLabError(error: unknown): ParsedGitLabError { return parseGitLabErrorMessage(error); } + // Handle Error-like objects with a message property + if (typeof error === 'object' && error !== null && 'message' in error) { + const message = (error as { message: string }).message; + if (typeof message === 'string') { + return parseGitLabErrorMessage(message); + } + } + return { code: GitLabErrorCode.UNKNOWN, recoverable: false @@ -45,8 +53,31 @@ export function parseGitLabError(error: unknown): ParsedGitLabError { function parseGitLabErrorMessage(message: string): ParsedGitLabError { const lowerMessage = message.toLowerCase(); + // Check for explicit HTTP status code in response (if available) + // Try to extract status from common patterns like "Status: 401" or HTTP error responses + const statusMatch = message.match(/\bstatus:\s*(\d{3})\b/i) || + message.match(/\bhttp\s+(\d{3})\b/i) || + lowerMessage.match(/\b"status":\s*(\d{3})\b/); + + if (statusMatch) { + const statusCode = parseInt(statusMatch[1], 10); + switch (statusCode) { + case 401: + return { code: GitLabErrorCode.AUTHENTICATION_FAILED, recoverable: true }; + case 403: + return { code: GitLabErrorCode.INSUFFICIENT_PERMISSIONS, recoverable: true }; + case 404: + return { code: GitLabErrorCode.PROJECT_NOT_FOUND, recoverable: true }; + case 409: + return { code: GitLabErrorCode.CONFLICT, recoverable: false }; + case 429: + return { code: GitLabErrorCode.RATE_LIMITED, recoverable: true }; + } + } + + // Fallback to message content analysis with word-boundary regex to avoid false matches // Authentication errors - if (lowerMessage.includes('401') || lowerMessage.includes('unauthorized') || lowerMessage.includes('invalid token')) { + if (/\b401\b/.test(message) || lowerMessage.includes('unauthorized') || lowerMessage.includes('invalid token')) { return { code: GitLabErrorCode.AUTHENTICATION_FAILED, recoverable: true @@ -54,7 +85,7 @@ function parseGitLabErrorMessage(message: string): ParsedGitLabError { } // Rate limiting (429) - if (lowerMessage.includes('429') || lowerMessage.includes('rate limit') || lowerMessage.includes('too many requests')) { + if (/\b429\b/.test(message) || lowerMessage.includes('rate limit') || lowerMessage.includes('too many requests')) { return { code: GitLabErrorCode.RATE_LIMITED, recoverable: true @@ -70,7 +101,7 @@ function parseGitLabErrorMessage(message: string): ParsedGitLabError { } // Project not found (404) - if (lowerMessage.includes('404') || lowerMessage.includes('not found')) { + if (/\b404\b/.test(message) || lowerMessage.includes('not found')) { return { code: GitLabErrorCode.PROJECT_NOT_FOUND, recoverable: true @@ -78,7 +109,7 @@ function parseGitLabErrorMessage(message: string): ParsedGitLabError { } // Permission denied (403) - if (lowerMessage.includes('403') || lowerMessage.includes('forbidden') || lowerMessage.includes('permission denied')) { + if (/\b403\b/.test(message) || lowerMessage.includes('forbidden') || lowerMessage.includes('permission denied')) { return { code: GitLabErrorCode.INSUFFICIENT_PERMISSIONS, recoverable: true @@ -86,7 +117,7 @@ function parseGitLabErrorMessage(message: string): ParsedGitLabError { } // Conflict (409) - if (lowerMessage.includes('409') || lowerMessage.includes('conflict')) { + if (/\b409\b/.test(message) || lowerMessage.includes('conflict')) { return { code: GitLabErrorCode.CONFLICT, recoverable: false diff --git a/apps/desktop/src/renderer/components/gitlab-merge-requests/components/MRLogs.tsx b/apps/desktop/src/renderer/components/gitlab-merge-requests/components/MRLogs.tsx index be10c193..d9af0aea 100644 --- a/apps/desktop/src/renderer/components/gitlab-merge-requests/components/MRLogs.tsx +++ b/apps/desktop/src/renderer/components/gitlab-merge-requests/components/MRLogs.tsx @@ -24,15 +24,15 @@ import { Clock, Activity } from 'lucide-react'; -import { Badge } from '../../ui/badge'; -import { Collapsible, CollapsibleTrigger, CollapsibleContent } from '../../ui/collapsible'; -import { cn } from '../../../lib/utils'; +import { Badge } from '@/components/ui/badge'; +import { Collapsible, CollapsibleTrigger, CollapsibleContent } from '@/components/ui/collapsible'; +import { cn } from '@/lib/utils'; import type { PRLogs, PRLogPhase, PRPhaseLog, PRLogEntry -} from '../../../../preload/api/modules/github-api'; +} from '@preload/api/modules/github-api'; // Type aliases for GitLab compatibility type GitLabMRLogs = PRLogs; diff --git a/apps/desktop/src/renderer/components/gitlab-merge-requests/components/StatusIndicator.tsx b/apps/desktop/src/renderer/components/gitlab-merge-requests/components/StatusIndicator.tsx index 5ba232e9..bab458d9 100644 --- a/apps/desktop/src/renderer/components/gitlab-merge-requests/components/StatusIndicator.tsx +++ b/apps/desktop/src/renderer/components/gitlab-merge-requests/components/StatusIndicator.tsx @@ -1,7 +1,7 @@ import { CheckCircle2, Circle, XCircle, Loader2, AlertTriangle, GitMerge, HelpCircle } from 'lucide-react'; -import { Badge } from '../../ui/badge'; -import { cn } from '../../../lib/utils'; -import type { ChecksStatus, ReviewsStatus, MergeableState } from '../../../../shared/types/pr-status'; +import { Badge } from '@/components/ui/badge'; +import { cn } from '@/lib/utils'; +import type { ChecksStatus, ReviewsStatus, MergeableState } from '@shared/types/pr-status'; import { useTranslation } from 'react-i18next'; /** diff --git a/apps/desktop/src/renderer/components/gitlab-merge-requests/hooks/useGitLabMRFiltering.ts b/apps/desktop/src/renderer/components/gitlab-merge-requests/hooks/useGitLabMRFiltering.ts index 0bd1cf63..222f44dd 100644 --- a/apps/desktop/src/renderer/components/gitlab-merge-requests/hooks/useGitLabMRFiltering.ts +++ b/apps/desktop/src/renderer/components/gitlab-merge-requests/hooks/useGitLabMRFiltering.ts @@ -3,6 +3,10 @@ * * Stub hook - implements the same pattern as usePRFiltering * adapted for GitLab merge requests. + * + * NOTE: This hook and MRFilterBar are reserved for future filtering functionality. + * They are not currently integrated into the GitLab MRs UI but are retained + * for when filtering/search features are implemented. */ import { useMemo, useState, useCallback } from 'react'; @@ -11,7 +15,7 @@ import type { GitLabMRReviewResult, GitLabMRReviewProgress, GitLabNewCommitsCheck -} from '../../../../shared/types'; +} from '@shared/types'; export type GitLabMRStatusFilter = | 'all' diff --git a/apps/desktop/src/renderer/stores/gitlab/__tests__/issues-store.test.ts b/apps/desktop/src/renderer/stores/gitlab/__tests__/issues-store.test.ts index 90a6f2cd..2e161dc9 100644 --- a/apps/desktop/src/renderer/stores/gitlab/__tests__/issues-store.test.ts +++ b/apps/desktop/src/renderer/stores/gitlab/__tests__/issues-store.test.ts @@ -88,20 +88,6 @@ describe('issues-store', () => { expect(selected?.iid).toBe(1); }); - it('should get filtered issues', () => { - const issues = [ - createMockGitLabIssue({ iid: 1, state: 'opened' }), - createMockGitLabIssue({ iid: 2, state: 'closed' }), - createMockGitLabIssue({ iid: 3, state: 'opened' }), - ]; - useIssuesStore.getState().setIssues(issues); - useIssuesStore.getState().setFilterState('opened'); - - const filtered = useIssuesStore.getState().getFilteredIssues(); - expect(filtered).toHaveLength(2); - expect(filtered.every((i: GitLabIssue) => i.state === 'opened')).toBe(true); - }); - it('should count open issues', () => { const issues = [ createMockGitLabIssue({ iid: 1, state: 'opened' }), diff --git a/apps/desktop/src/renderer/stores/gitlab/__tests__/sync-status-store.test.ts b/apps/desktop/src/renderer/stores/gitlab/__tests__/sync-status-store.test.ts index 79da5132..8e586c6a 100644 --- a/apps/desktop/src/renderer/stores/gitlab/__tests__/sync-status-store.test.ts +++ b/apps/desktop/src/renderer/stores/gitlab/__tests__/sync-status-store.test.ts @@ -106,6 +106,22 @@ describe('sync-status-store', () => { expect(useSyncStatusStore.getState().connectionError).toBe('Authentication failed'); }); + it('should set error when connected is false', async () => { + mockElectronAPI.checkGitLabConnection.mockResolvedValue({ + success: true, + data: { + connected: false, + error: 'Project not found' + } + }); + + const result = await checkGitLabConnection('project-123'); + + expect(result).toBe(null); + expect(useSyncStatusStore.getState().syncStatus).toBe(null); + expect(useSyncStatusStore.getState().connectionError).toBe('Project not found'); + }); + it('should set error on exception', async () => { mockElectronAPI.checkGitLabConnection.mockRejectedValue(new Error('Network error')); diff --git a/apps/desktop/src/renderer/stores/gitlab/issues-store.ts b/apps/desktop/src/renderer/stores/gitlab/issues-store.ts index b9ef02e2..9c3f4ed2 100644 --- a/apps/desktop/src/renderer/stores/gitlab/issues-store.ts +++ b/apps/desktop/src/renderer/stores/gitlab/issues-store.ts @@ -119,8 +119,8 @@ export async function loadGitLabIssues( try { const result = await window.electronAPI.getGitLabIssues(projectId, state); - // Guard against stale responses - if (store.currentRequestToken !== requestId) { + // Guard against stale responses - read live state, not captured store reference + if (useIssuesStore.getState().currentRequestToken !== requestId) { return; // A newer request has superseded this one } @@ -130,14 +130,14 @@ export async function loadGitLabIssues( store.setError(result.error || 'Failed to load GitLab issues'); } } catch (error) { - // Guard against stale responses in error case - if (store.currentRequestToken !== requestId) { + // Guard against stale responses in error case - read live state + if (useIssuesStore.getState().currentRequestToken !== requestId) { return; } store.setError(error instanceof Error ? error.message : 'Unknown error'); } finally { - // Only clear loading state if this is still the current request - if (store.currentRequestToken === requestId) { + // Only clear loading state if this is still the current request - read live state + if (useIssuesStore.getState().currentRequestToken === requestId) { store.setLoading(false); } } diff --git a/apps/desktop/src/renderer/stores/gitlab/sync-status-store.ts b/apps/desktop/src/renderer/stores/gitlab/sync-status-store.ts index 88bbf671..02549149 100644 --- a/apps/desktop/src/renderer/stores/gitlab/sync-status-store.ts +++ b/apps/desktop/src/renderer/stores/gitlab/sync-status-store.ts @@ -57,9 +57,15 @@ export async function checkGitLabConnection(projectId: string): Promise