diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f11c87f..5c44917 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -117,10 +117,10 @@ jobs: url: ${{ steps.deployment.outputs.page_url }} steps: - - name: Checkout correct version + - name: Checkout main branch uses: actions/checkout@v4 with: - ref: v${{ needs.release.outputs.version }} + ref: main fetch-depth: 0 - name: Setup Node.js diff --git a/apps/desktop/src-tauri/src/git.rs b/apps/desktop/src-tauri/src/git.rs index c89f9f8..343fca9 100644 --- a/apps/desktop/src-tauri/src/git.rs +++ b/apps/desktop/src-tauri/src/git.rs @@ -13,7 +13,9 @@ pub struct LoadRepoResult { pub struct DiffFile { pub path: String, #[serde(rename = "type")] - pub change_type: String, // "modify" | "add" | "remove" + pub change_type: String, // "modify" | "add" | "remove" | "rename" | "copy" + #[serde(skip_serializing_if = "Option::is_none")] + pub old_path: Option, // For renames and copies } #[derive(Debug, Serialize, Deserialize)] @@ -108,32 +110,53 @@ pub fn git_diff(path: &str, base: &str, compare: &str) -> Result "add", - git2::Delta::Deleted => "remove", - git2::Delta::Modified => "modify", - git2::Delta::Renamed => "modify", - git2::Delta::Copied => "add", - _ => "modify", + .map(|s| s.to_string()); + + let (path, change_type, stored_old_path) = match delta.status() { + git2::Delta::Added => { + (new_path.unwrap_or_default(), "add", None) + }, + git2::Delta::Deleted => { + (old_path.unwrap_or_default(), "remove", None) + }, + git2::Delta::Modified => { + (new_path.unwrap_or_default(), "modify", None) + }, + git2::Delta::Renamed => { + // For renames, store the new path as primary and old path separately + (new_path.clone().unwrap_or_default(), "rename", old_path) + }, + git2::Delta::Copied => { + // For copies, store the new path as primary and old path separately + (new_path.clone().unwrap_or_default(), "copy", old_path) + }, + _ => { + (new_path.or(old_path).unwrap_or_default(), "modify", None) + }, }; files.push(DiffFile { path, change_type: change_type.to_string(), + old_path: stored_old_path, }); true @@ -297,9 +320,8 @@ pub fn read_file_blob(path: &str, ref_name: &str, file_path: &str) -> Result void) { }, [currentDir, loadRepoFromHandle]) const resetRepo = useCallback(() => { + // Note: dispose() clears the repo path but the service instance remains + // valid and can be reused. This is by design for the singleton pattern. gitClient.dispose() setRepoStatus({ state: 'idle' }) setCurrentDir(null) diff --git a/apps/desktop/src/services/TauriGitService.ts b/apps/desktop/src/services/TauriGitService.ts index 83f30e7..f19524f 100644 --- a/apps/desktop/src/services/TauriGitService.ts +++ b/apps/desktop/src/services/TauriGitService.ts @@ -111,6 +111,11 @@ export class TauriGitService implements GitService { return result } + /** + * Dispose of resources by clearing the repo path. + * Note: This service instance remains valid and can be reused after dispose(). + * Call loadRepo() again to load a new repository. + */ dispose(): void { this.repoPath = null } diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 52796de..1434974 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -21,6 +21,34 @@ import { clearRepositoryCache } from './utils/cache' import { logError } from './utils/logger' import { debounce } from './utils/debounce' +// Prompt templates (moved outside component to avoid recreation on every render) +const PROMPT_TEMPLATES = [ + { + id: 'branch-summary', + label: 'Summarize branch diff', + content: + 'You are an expert engineer. Summarize the changes between the selected branches and explain the impact of the modifications.', + }, + { + id: 'wd-review', + label: 'Review working directory changes', + content: + 'You are a code reviewer. Review the current working directory diff for potential issues, bugs, or improvements.', + }, + { + id: 'test-plan', + label: 'Suggest tests for diff', + content: + 'You are a QA engineer. Based on the provided diff, propose relevant unit or integration tests to cover the changes.', + }, + { + id: 'release-notes', + label: 'Draft release notes', + content: + 'You are a technical writer. Craft concise release notes that describe the user-facing effects of the diff.', + }, +] + function App() { const [appStatus, setAppStatus] = useState({ state: 'IDLE' }) // note: we will temporarily set task='tokens' while counting, see effect below @@ -51,6 +79,9 @@ function App() { compare?: { binary: boolean; text: string | null; notFound?: boolean } } | null>(null) + // Track preview request IDs to prevent race conditions + const previewRequestIdRef = useRef(0) + const [theme, setTheme] = useState<'light' | 'dark' | null>(() => { try { const saved = localStorage.getItem('gc.theme') @@ -270,32 +301,6 @@ function App() { return () => { cancelled = true } }, [userInstructions]) - const PROMPT_TEMPLATES = [ - { - id: 'branch-summary', - label: 'Summarize branch diff', - content: - 'You are an expert engineer. Summarize the changes between the selected branches and explain the impact of the modifications.', - }, - { - id: 'wd-review', - label: 'Review working directory changes', - content: - 'You are a code reviewer. Review the current working directory diff for potential issues, bugs, or improvements.', - }, - { - id: 'test-plan', - label: 'Suggest tests for diff', - content: - 'You are a QA engineer. Based on the provided diff, propose relevant unit or integration tests to cover the changes.', - }, - { - id: 'release-notes', - label: 'Draft release notes', - content: - 'You are a technical writer. Craft concise release notes that describe the user-facing effects of the diff.', - }, - ] const [templateId, setTemplateId] = useState('') // Model selection: fetched dynamically; derive token limit from the selected model @@ -370,6 +375,14 @@ function App() { const diffRangeRef = useRef(null) const MAX_CONTEXT = 999 const debouncedSetDiffContextLines = useMemo(() => debounce(setDiffContextLines, 250), []) + + // Cancel debounced function on unmount to avoid setState after unmount + useEffect(() => { + return () => { + debouncedSetDiffContextLines.cancel() + } + }, [debouncedSetDiffContextLines]) + // Collapsible User Instructions const [instructionsOpen, setInstructionsOpen] = useState(true) @@ -583,6 +596,10 @@ function App() { setNotif('Binary file preview is not supported.') return } + + // Increment request ID to track this specific preview request + const requestId = ++previewRequestIdRef.current + try { const toFetchBase = status !== 'add' const toFetchCompare = status !== 'remove' @@ -592,6 +609,11 @@ function App() { toFetchCompare && compareBranch ? gitClient.readFile(compareBranch, path) : Promise.resolve(undefined), ]) + // Check if this request is still the latest one (prevent race condition) + if (requestId !== previewRequestIdRef.current) { + return // A newer preview request has been made, ignore this result + } + // If worker reports binary, bail out as well const baseBin = (baseRes as any)?.binary const compareBin = (compareRes as any)?.binary @@ -607,8 +629,11 @@ function App() { }) setPreviewOpen(true) } catch (e) { - const err = e instanceof Error ? e : new Error(String(e)) - setNotif(`Failed to read file content: ${err.message}`) + // Only show error if this is still the latest request + if (requestId === previewRequestIdRef.current) { + const err = e instanceof Error ? e : new Error(String(e)) + setNotif(`Failed to read file content: ${err.message}`) + } } } @@ -1151,9 +1176,24 @@ function App() { value={templateId} onChange={(e) => { const id = e.target.value - setTemplateId(id) const tmpl = PROMPT_TEMPLATES.find((t) => t.id === id) - if (tmpl) setUserInstructions(tmpl.content) + if (tmpl) { + // If user has custom instructions, confirm before overwriting + const hasCustomText = userInstructions.trim() && userInstructions.trim() !== tmpl.content + if (hasCustomText) { + const confirmed = window.confirm( + 'This will replace your current instructions. Continue?' + ) + if (!confirmed) { + // Reset select to previous value + return + } + } + setTemplateId(id) + setUserInstructions(tmpl.content) + } else { + setTemplateId(id) + } }} style={{ flexGrow: 1 }} > @@ -1297,7 +1337,6 @@ function App() {
toggleSelect(path)} diff --git a/apps/web/src/hooks/useFileTree.ts b/apps/web/src/hooks/useFileTree.ts index a3df2d7..29ad36e 100644 --- a/apps/web/src/hooks/useFileTree.ts +++ b/apps/web/src/hooks/useFileTree.ts @@ -1,4 +1,4 @@ -import { useCallback, useState } from 'react' +import { useCallback, useState, useRef } from 'react' import type { GitWorkerClient } from '../utils/gitWorkerClient' import type { AppStatus } from '../types/appStatus' import { isBinaryPath, LARGE_REPO_FILE_THRESHOLD, type FileDiffStatus, type FileTreeNode } from '@gitcontext/core' @@ -18,6 +18,9 @@ export function useFileTree(setAppStatus?: (s: AppStatus) => void) { const [selectedPaths, setSelectedPaths] = useState>(new Set()) const [isComputing, setIsComputing] = useState(false) + // Track diff computation request IDs to prevent race conditions + const diffRequestIdRef = useRef(0) + const buildTreeFromPaths = useCallback((allPaths: string[], diffMap: Map): { tree: FileTreeNode; statusByPath: Map } => { const root: FileTreeNode = { name: '', path: '', type: 'dir', children: [] } const dirMap = new Map() @@ -85,14 +88,23 @@ export function useFileTree(setAppStatus?: (s: AppStatus) => void) { setExpandedPaths(new Set()) return } + + // Increment request ID to track this specific diff computation + const requestId = ++diffRequestIdRef.current setIsComputing(true) - + try { setAppStatus?.({ state: 'LOADING', task: 'diff', message: 'Computing file differences…', progress: 25 }) try { console.info('[app-status]', { state: 'LOADING', task: 'diff', message: 'Computing file differences…', progress: 25 }) } catch {} setProgress?.({ message: 'Computing file differences…', percent: 25 }) - + const res = await gitClient.diff(baseBranch, compareBranch) + + // Check if this request is still the latest one (prevent race condition) + if (requestId !== diffRequestIdRef.current) { + return // A newer diff request has been made, ignore this result + } + setDiffFiles(res.files) setProgress?.({ message: 'Fetching file list…', percent: 50 }) @@ -100,6 +112,11 @@ export function useFileTree(setAppStatus?: (s: AppStatus) => void) { try { console.info('[app-status]', { state: 'LOADING', task: 'diff', message: 'Fetching file list…', progress: 50 }) } catch {} const baseList = await gitClient.listFiles(baseBranch) const compareList = await gitClient.listFiles(compareBranch) + + // Check again after async operations + if (requestId !== diffRequestIdRef.current) { + return + } const diffMap = new Map() for (const f of res.files) diffMap.set(f.path, f.type as FileDiffStatus) // Build union from both sides to keep unchanged files present on either side @@ -108,6 +125,12 @@ export function useFileTree(setAppStatus?: (s: AppStatus) => void) { setAppStatus?.({ state: 'LOADING', task: 'diff', message: 'Building file tree…', progress: 75 }) try { console.info('[app-status]', { state: 'LOADING', task: 'diff', message: 'Building file tree…', progress: 75 }) } catch {} const { tree, statusByPath: statusMap } = buildTreeFromPaths(Array.from(union), diffMap) + + // Final check before committing all state updates + if (requestId !== diffRequestIdRef.current) { + return + } + setFileTree(tree) setStatusByPath(statusMap) diff --git a/apps/web/src/hooks/useTokenCounts.ts b/apps/web/src/hooks/useTokenCounts.ts index ad4017e..4bee778 100644 --- a/apps/web/src/hooks/useTokenCounts.ts +++ b/apps/web/src/hooks/useTokenCounts.ts @@ -5,6 +5,20 @@ import type { FileDiffStatus } from './useFileTree' import { buildUnifiedDiffForStatus } from '../utils/diff' import { isBinaryPath, MAX_CONCURRENT_READS } from '@gitcontext/core' import { mapWithConcurrency } from '../utils/concurrency' +import { logError } from '../utils/logger' + +// Helper to infer language from file extension for syntax highlighting +function inferLangFromPath(p: string): string { + const ext = p.split('.').pop()?.toLowerCase() || '' + const langMap: Record = { + js: 'javascript', jsx: 'javascript', ts: 'typescript', tsx: 'typescript', + py: 'python', rb: 'ruby', go: 'go', rs: 'rust', c: 'c', cpp: 'cpp', java: 'java', + kt: 'kotlin', swift: 'swift', php: 'php', cs: 'csharp', sh: 'bash', yaml: 'yaml', + yml: 'yaml', json: 'json', xml: 'xml', html: 'html', css: 'css', scss: 'scss', + md: 'markdown', sql: 'sql', + } + return langMap[ext] || '' +} export type TokenCounts = Map @@ -80,28 +94,42 @@ export function useTokenCounts({ needBase && baseRef ? gitClient.readFile(baseRef, path) : Promise.resolve(undefined as any), needCompare && compareRef ? gitClient.readFile(compareRef, path) : Promise.resolve(undefined as any), ]) - // Mirror final output generation logic + // Mirror final output generation logic EXACTLY as in copyAllSelected + const header = `## FILE: ${path} (${(status || 'unchanged').toUpperCase()})\n\n` const MAX_CONTEXT = 999 const ctx = diffContextLines >= MAX_CONTEXT ? Number.MAX_SAFE_INTEGER : diffContextLines + if (status === 'modify' || status === 'add' || status === 'remove') { const isBinary = Boolean((baseRes as any)?.binary) || Boolean((compareRes as any)?.binary) if (isBinary) { // Edge: unknown ext but worker says binary; treat same as looksBinary if (includeBinaryPaths) { - const header = `## FILE: ${path} (${(status || 'unchanged').toUpperCase()})\n\n` - textForCount = header + textForCount = header // Just the header, no [Binary file] text } else { textForCount = '' } - } else if (status === 'add' && ctx === Number.MAX_SAFE_INTEGER) { - textForCount = (compareRes as { text?: string } | undefined)?.text ?? '' + } else if (status === 'add') { + // Include header + markdown fences + content (matches copyAllSelected line 781) + const newTextRaw = (compareRes as { text?: string } | undefined)?.text ?? '' + const newText = newTextRaw.endsWith('\n') ? newTextRaw.slice(0, -1) : newTextRaw + const lang = inferLangFromPath(path) + textForCount = header + '```' + lang + '\n' + newText + '\n```\n\n' } else { - textForCount = buildUnifiedDiffForStatus(status, path, baseRes as any, compareRes as any, { context: ctx }) || '' + // modify/remove: include header + diff fences + diff (matches copyAllSelected line 791) + const diffText = buildUnifiedDiffForStatus(status, path, baseRes as any, compareRes as any, { context: ctx }) || '' + if (diffText) { + textForCount = header + '```diff\n' + diffText + '```\n\n' + } else { + // Fallback: no text (matches copyAllSelected line 794) + textForCount = header + '_No textual content available._\n\n' + } } } else { + // unchanged: include header + markdown fences + content (matches copyAllSelected line 800) const isBinary = Boolean((baseRes as any)?.binary) - const oldText = isBinary || (baseRes as any)?.notFound ? '' : (baseRes as any)?.text ?? '' - textForCount = oldText + const text = isBinary || (baseRes as any)?.notFound ? '' : (baseRes as any)?.text ?? '' + const lang = inferLangFromPath(path) + textForCount = header + '```' + lang + '\n' + (text || '') + '\n```\n\n' } } const n = textForCount ? await tok.count(textForCount) : 0 @@ -125,8 +153,10 @@ export function useTokenCounts({ } setCounts(next) } catch (err: any) { + // Don't throw from effect to avoid unhandled promise rejection if (err?.message !== 'Operation cancelled') { - throw err + logError('tokenCounts', err) + setCounts(new Map()) // Reset to empty on error } } finally { setBusy(false) @@ -140,7 +170,7 @@ export function useTokenCounts({ return () => { abortController.abort() } - }, [gitClient, baseRef, compareRef, selectedList, statusByPath, diffContextLines]) + }, [gitClient, baseRef, compareRef, selectedList, statusByPath, diffContextLines, includeBinaryPaths, tok, onBatch]) const total = useMemo(() => { let sum = 0 diff --git a/apps/web/src/utils/debounce.ts b/apps/web/src/utils/debounce.ts index f29a514..ba3eaf7 100644 --- a/apps/web/src/utils/debounce.ts +++ b/apps/web/src/utils/debounce.ts @@ -3,14 +3,25 @@ export function debounce unknown>( delay: number, ) { let timeoutId: ReturnType | null = null - return function(this: ThisParameterType, ...args: Parameters) { + + const debounced = function(this: ThisParameterType, ...args: Parameters) { if (timeoutId) { clearTimeout(timeoutId) } timeoutId = setTimeout(() => { fn.apply(this, args as unknown as []); + timeoutId = null }, delay) + } as T & { cancel: () => void } + + debounced.cancel = () => { + if (timeoutId) { + clearTimeout(timeoutId) + timeoutId = null + } } + + return debounced }