From 63f46173544234f55e238a31ddd36cce962f6a71 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Mon, 5 Jan 2026 14:40:24 +0100 Subject: [PATCH] sentry dev support + sessions handling in terminals --- apps/frontend/package.json | 1 + apps/frontend/src/main/index.ts | 24 ++++- .../terminal/claude-integration-handler.ts | 2 + .../src/main/terminal/session-handler.ts | 100 +++++++++++++++--- .../src/main/terminal/terminal-lifecycle.ts | 14 ++- .../components/ReviewStatusTree.tsx | 15 ++- .../components/terminal/usePtyProcess.ts | 13 ++- .../src/shared/i18n/locales/en/common.json | 1 + .../src/shared/i18n/locales/fr/common.json | 1 + package-lock.json | 2 +- 10 files changed, 151 insertions(+), 22 deletions(-) diff --git a/apps/frontend/package.json b/apps/frontend/package.json index 07b19c2d..15bacd16 100644 --- a/apps/frontend/package.json +++ b/apps/frontend/package.json @@ -80,6 +80,7 @@ "chokidar": "^5.0.0", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", + "dotenv": "^16.6.1", "electron-log": "^5.4.3", "electron-updater": "^6.6.2", "i18next": "^25.7.3", diff --git a/apps/frontend/src/main/index.ts b/apps/frontend/src/main/index.ts index 3a96c241..bbb92f5f 100644 --- a/apps/frontend/src/main/index.ts +++ b/apps/frontend/src/main/index.ts @@ -1,6 +1,28 @@ +// Load .env file FIRST before any other imports that might use process.env +import { config } from 'dotenv'; +import { resolve, dirname } from 'path'; +import { existsSync } from 'fs'; + +// Load .env from apps/frontend directory +// In development: __dirname is out/main (compiled), so go up 2 levels +// In production: app resources directory +const possibleEnvPaths = [ + resolve(__dirname, '../../.env'), // Development: out/main -> apps/frontend/.env + resolve(__dirname, '../../../.env'), // Alternative: might be in different location + resolve(process.cwd(), 'apps/frontend/.env'), // Fallback: from workspace root +]; + +for (const envPath of possibleEnvPaths) { + if (existsSync(envPath)) { + config({ path: envPath }); + console.log(`[dotenv] Loaded environment from: ${envPath}`); + break; + } +} + import { app, BrowserWindow, shell, nativeImage, session } from 'electron'; import { join } from 'path'; -import { accessSync, readFileSync, writeFileSync, rmSync, existsSync } from 'fs'; +import { accessSync, readFileSync, writeFileSync, rmSync } from 'fs'; import { electronApp, optimizer, is } from '@electron-toolkit/utils'; import { setupIpcHandlers } from './ipc-setup'; import { AgentManager } from './agent'; diff --git a/apps/frontend/src/main/terminal/claude-integration-handler.ts b/apps/frontend/src/main/terminal/claude-integration-handler.ts index 0a72c322..0949a425 100644 --- a/apps/frontend/src/main/terminal/claude-integration-handler.ts +++ b/apps/frontend/src/main/terminal/claude-integration-handler.ts @@ -211,6 +211,8 @@ export function invokeClaude( debugLog('[ClaudeIntegration:invokeClaude] CWD:', cwd); terminal.isClaudeMode = true; + // Release any previously claimed session ID before starting new session + SessionHandler.releaseSessionId(terminal.id); terminal.claudeSessionId = undefined; const startTime = Date.now(); diff --git a/apps/frontend/src/main/terminal/session-handler.ts b/apps/frontend/src/main/terminal/session-handler.ts index f26a8965..38edfa1e 100644 --- a/apps/frontend/src/main/terminal/session-handler.ts +++ b/apps/frontend/src/main/terminal/session-handler.ts @@ -11,6 +11,48 @@ import { getTerminalSessionStore, type TerminalSession } from '../terminal-sessi import { IPC_CHANNELS } from '../../shared/constants'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; +/** + * Track session IDs that have been claimed by terminals to prevent race conditions. + * When multiple terminals invoke Claude simultaneously, this prevents them from + * all capturing the same session ID. + * + * Key: sessionId, Value: terminalId that claimed it + */ +const claimedSessionIds: Map = new Map(); + +/** + * Claim a session ID for a terminal. Returns true if successful, false if already claimed. + */ +export function claimSessionId(sessionId: string, terminalId: string): boolean { + const existingClaim = claimedSessionIds.get(sessionId); + if (existingClaim && existingClaim !== terminalId) { + debugLog('[SessionHandler] Session ID already claimed:', sessionId, 'by terminal:', existingClaim); + return false; + } + claimedSessionIds.set(sessionId, terminalId); + debugLog('[SessionHandler] Claimed session ID:', sessionId, 'for terminal:', terminalId); + return true; +} + +/** + * Release a session ID claim when a terminal is destroyed or session changes. + */ +export function releaseSessionId(terminalId: string): void { + for (const [sessionId, claimedBy] of claimedSessionIds.entries()) { + if (claimedBy === terminalId) { + claimedSessionIds.delete(sessionId); + debugLog('[SessionHandler] Released session ID:', sessionId, 'from terminal:', terminalId); + } + } +} + +/** + * Get all currently claimed session IDs (for exclusion during search). + */ +export function getClaimedSessionIds(): Set { + return new Set(claimedSessionIds.keys()); +} + /** * Get the Claude project slug from a project path. * Claude uses the full path with forward slashes replaced by dashes. @@ -56,9 +98,19 @@ export function findMostRecentClaudeSession(projectPath: string): string | null } /** - * Find a Claude session created/modified after a given timestamp + * Find a Claude session created/modified after a given timestamp. + * Excludes session IDs that have already been claimed by other terminals + * to prevent race conditions when multiple terminals invoke Claude simultaneously. + * + * @param projectPath - The project path to search sessions for + * @param afterTimestamp - Only consider sessions modified after this timestamp + * @param excludeSessionIds - Optional set of session IDs to exclude (already claimed) */ -export function findClaudeSessionAfter(projectPath: string, afterTimestamp: number): string | null { +export function findClaudeSessionAfter( + projectPath: string, + afterTimestamp: number, + excludeSessionIds?: Set +): string | null { const slug = getClaudeProjectSlug(projectPath); const claudeProjectDir = path.join(os.homedir(), '.claude', 'projects', slug); @@ -71,17 +123,22 @@ export function findClaudeSessionAfter(projectPath: string, afterTimestamp: numb .filter(f => f.endsWith('.jsonl')) .map(f => ({ name: f, + sessionId: f.replace('.jsonl', ''), path: path.join(claudeProjectDir, f), mtime: fs.statSync(path.join(claudeProjectDir, f)).mtime.getTime() })) .filter(f => f.mtime > afterTimestamp) + // Exclude already-claimed session IDs to prevent race conditions + .filter(f => !excludeSessionIds || !excludeSessionIds.has(f.sessionId)) .sort((a, b) => b.mtime - a.mtime); if (files.length === 0) { return null; } - return files[0].name.replace('.jsonl', ''); + const sessionId = files[0].sessionId; + debugLog('[SessionHandler] Found unclaimed session after timestamp:', sessionId, 'excluded:', excludeSessionIds?.size ?? 0); + return sessionId; } catch (error) { debugError('[SessionHandler] Error finding Claude session:', error); return null; @@ -184,7 +241,9 @@ export function getSessionsForDate(date: string, projectPath: string): TerminalS } /** - * Attempt to capture Claude session ID by polling the session directory + * Attempt to capture Claude session ID by polling the session directory. + * Uses the claim mechanism to prevent race conditions when multiple terminals + * invoke Claude simultaneously - each terminal will get a unique session ID. */ export function captureClaudeSessionId( terminalId: string, @@ -201,31 +260,44 @@ export function captureClaudeSessionId( const terminal = terminals.get(terminalId); if (!terminal || !terminal.isClaudeMode) { + debugLog('[SessionHandler] Terminal no longer in Claude mode, stopping session capture:', terminalId); return; } if (terminal.claudeSessionId) { + debugLog('[SessionHandler] Terminal already has session ID, stopping capture:', terminalId); return; } - const sessionId = findClaudeSessionAfter(projectPath, startTime); + // Get currently claimed session IDs to exclude from search + const claimedIds = getClaimedSessionIds(); + const sessionId = findClaudeSessionAfter(projectPath, startTime, claimedIds); if (sessionId) { - terminal.claudeSessionId = sessionId; - debugLog('[SessionHandler] Captured Claude session ID from directory:', sessionId); + // Try to claim this session ID - if another terminal beat us to it, keep searching + if (claimSessionId(sessionId, terminalId)) { + terminal.claudeSessionId = sessionId; + debugLog('[SessionHandler] Captured and claimed Claude session ID:', sessionId, 'for terminal:', terminalId); - if (terminal.projectPath) { - updateClaudeSessionId(terminal.projectPath, terminalId, sessionId); - } + if (terminal.projectPath) { + updateClaudeSessionId(terminal.projectPath, terminalId, sessionId); + } - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_SESSION, terminalId, sessionId); + const win = getWindow(); + if (win) { + win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_SESSION, terminalId, sessionId); + } + } else { + // Session was claimed by another terminal, keep polling for a different one + debugLog('[SessionHandler] Session ID was claimed by another terminal, continuing to poll:', sessionId); + if (attempts < maxAttempts) { + setTimeout(checkForSession, 1000); + } } } else if (attempts < maxAttempts) { setTimeout(checkForSession, 1000); } else { - debugLog('[SessionHandler] Could not capture Claude session ID after', maxAttempts, 'attempts'); + debugLog('[SessionHandler] Could not capture Claude session ID after', maxAttempts, 'attempts for terminal:', terminalId); } }; diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index 51544722..7f0ceebe 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -57,8 +57,16 @@ export async function createTerminal( debugLog('[TerminalLifecycle] Injecting OAuth token from active profile'); } + // Validate cwd exists - if the directory doesn't exist (e.g., worktree removed), + // fall back to project path to prevent shell exit with code 1 + let effectiveCwd = cwd; + if (cwd && !existsSync(cwd)) { + debugLog('[TerminalLifecycle] Terminal cwd does not exist, falling back:', cwd, '->', projectPath || os.homedir()); + effectiveCwd = projectPath || os.homedir(); + } + const ptyProcess = PtyManager.spawnPtyProcess( - cwd || os.homedir(), + effectiveCwd || os.homedir(), cols, rows, profileEnv @@ -66,7 +74,7 @@ export async function createTerminal( debugLog('[TerminalLifecycle] PTY process spawned, pid:', ptyProcess.pid); - const terminalCwd = cwd || os.homedir(); + const terminalCwd = effectiveCwd || os.homedir(); const terminal: TerminalProcess = { id, pty: ptyProcess, @@ -234,6 +242,8 @@ export async function destroyTerminal( try { SessionHandler.removePersistedSession(terminal); + // Release any claimed session ID for this terminal + SessionHandler.releaseSessionId(id); onCleanup(id); PtyManager.killPty(terminal); terminals.delete(id); diff --git a/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx b/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx index f9d81ac8..4dc8485f 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx @@ -1,5 +1,5 @@ import { useState } from 'react'; -import { CheckCircle, Circle, CircleDot, Play } from 'lucide-react'; +import { CheckCircle, Circle, CircleDot, Play, RefreshCw } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { Button } from '../../ui/button'; import { cn } from '../../../lib/utils'; @@ -143,7 +143,18 @@ export function ReviewStatusTree({ id: 'analysis', label: t('prReview.analysisComplete', { count: reviewResult.findings.length }), status: 'completed', - date: reviewResult.reviewedAt + date: reviewResult.reviewedAt, + action: ( + + ) }); } diff --git a/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts b/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts index e50f433c..d64d6945 100644 --- a/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts +++ b/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useCallback } from 'react'; +import { useEffect, useRef, useCallback, useState } from 'react'; import { useTerminalStore } from '../../stores/terminal-store'; interface UsePtyProcessOptions { @@ -25,6 +25,9 @@ export function usePtyProcess({ const isCreatingRef = useRef(false); const isCreatedRef = useRef(false); const currentCwdRef = useRef(cwd); + // Trigger state to force re-creation after resetForRecreate() + // Refs don't trigger re-renders, so we need a state to ensure the effect runs + const [recreationTrigger, setRecreationTrigger] = useState(0); const setTerminalStatus = useTerminalStore((state) => state.setTerminalStatus); const updateTerminal = useTerminalStore((state) => state.updateTerminal); @@ -45,6 +48,8 @@ export function usePtyProcess({ }, [cwd]); // Create PTY process + // recreationTrigger is included to force the effect to run after resetForRecreate() + // since refs don't trigger re-renders useEffect(() => { // Skip creation if explicitly told to (waiting for dimensions) if (skipCreation) return; @@ -111,7 +116,8 @@ export function usePtyProcess({ isCreatingRef.current = false; }); } - }, [terminalId, cwd, projectPath, cols, rows, skipCreation, setTerminalStatus, updateTerminal, onCreated, onError]); + + }, [terminalId, cwd, projectPath, cols, rows, skipCreation, recreationTrigger, setTerminalStatus, updateTerminal, onCreated, onError]); // Function to prepare for recreation by preventing the effect from running // Call this BEFORE updating the store cwd to avoid race condition @@ -121,9 +127,12 @@ export function usePtyProcess({ // Function to reset refs and allow recreation // Call this AFTER destroying the old terminal + // Increments recreationTrigger to force the effect to run since refs don't trigger re-renders const resetForRecreate = useCallback(() => { isCreatedRef.current = false; isCreatingRef.current = false; + // Increment trigger to force the creation effect to run + setRecreationTrigger((prev) => prev + 1); }, []); return { diff --git a/apps/frontend/src/shared/i18n/locales/en/common.json b/apps/frontend/src/shared/i18n/locales/en/common.json index ed25a1af..a84003a0 100644 --- a/apps/frontend/src/shared/i18n/locales/en/common.json +++ b/apps/frontend/src/shared/i18n/locales/en/common.json @@ -234,6 +234,7 @@ "followup": "Follow-up", "initial": "Initial", "rerunFollowup": "Re-run follow-up review", + "rerunReview": "Re-run review", "loadingMore": "Loading more PRs...", "scrollForMore": "Scroll for more", "allPRsLoaded": "All PRs loaded", diff --git a/apps/frontend/src/shared/i18n/locales/fr/common.json b/apps/frontend/src/shared/i18n/locales/fr/common.json index 0f96f221..a3664e3d 100644 --- a/apps/frontend/src/shared/i18n/locales/fr/common.json +++ b/apps/frontend/src/shared/i18n/locales/fr/common.json @@ -234,6 +234,7 @@ "followup": "Suivi", "initial": "Initial", "rerunFollowup": "Relancer la revue de suivi", + "rerunReview": "Relancer la revue", "loadingMore": "Chargement des PRs...", "scrollForMore": "Défiler pour plus", "allPRsLoaded": "Tous les PRs chargés", diff --git a/package-lock.json b/package-lock.json index 1d509585..7d729b30 100644 --- a/package-lock.json +++ b/package-lock.json @@ -58,6 +58,7 @@ "chokidar": "^5.0.0", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", + "dotenv": "^16.6.1", "electron-log": "^5.4.3", "electron-updater": "^6.6.2", "i18next": "^25.7.3", @@ -8265,7 +8266,6 @@ "version": "16.6.1", "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-16.6.1.tgz", "integrity": "sha512-uBq4egWHTcTt33a72vpSG0z3HnPuIl6NqYcTrKEg2azoEyl2hpW0zqlxysq2pK9HlDIHyHyakeYaYnSAwd8bow==", - "dev": true, "license": "BSD-2-Clause", "engines": { "node": ">=12"