From 47c188d8f9405996f3fce47f6bdc60fb8e7b8f59 Mon Sep 17 00:00:00 2001 From: Shirone Date: Mon, 12 Jan 2026 21:41:34 +0100 Subject: [PATCH] fix: adress pr comments - Added validation to check if the specified worktree path exists before generating commit messages. - Implemented a check to ensure the worktree path is a valid git repository by verifying the presence of the .git directory. - Improved error handling by returning appropriate responses for invalid paths and non-git repositories. --- .../routes/generate-commit-message.ts | 23 ++++++++++++ apps/ui/src/components/views/agent-view.tsx | 16 +++++---- .../components/list-view/list-row.tsx | 36 ------------------- .../views/graph-view/graph-canvas.tsx | 25 +++++++++++-- .../api-keys/claude-usage-section.tsx | 7 ++-- .../components/settings-navigation.tsx | 29 +++++++-------- 6 files changed, 75 insertions(+), 61 deletions(-) diff --git a/apps/server/src/routes/worktree/routes/generate-commit-message.ts b/apps/server/src/routes/worktree/routes/generate-commit-message.ts index 09e79641..eb959166 100644 --- a/apps/server/src/routes/worktree/routes/generate-commit-message.ts +++ b/apps/server/src/routes/worktree/routes/generate-commit-message.ts @@ -8,6 +8,8 @@ import type { Request, Response } from 'express'; import { exec } from 'child_process'; import { promisify } from 'util'; +import { existsSync } from 'fs'; +import { join } from 'path'; import { query } from '@anthropic-ai/claude-agent-sdk'; import { createLogger } from '@automaker/utils'; import { DEFAULT_PHASE_MODELS, isCursorModel, stripProviderPrefix } from '@automaker/types'; @@ -87,6 +89,27 @@ export function createGenerateCommitMessageHandler( return; } + // Validate that the directory exists + if (!existsSync(worktreePath)) { + const response: GenerateCommitMessageErrorResponse = { + success: false, + error: 'worktreePath does not exist', + }; + res.status(400).json(response); + return; + } + + // Validate that it's a git repository (check for .git folder or file for worktrees) + const gitPath = join(worktreePath, '.git'); + if (!existsSync(gitPath)) { + const response: GenerateCommitMessageErrorResponse = { + success: false, + error: 'worktreePath is not a git repository', + }; + res.status(400).json(response); + return; + } + logger.info(`Generating commit message for worktree: ${worktreePath}`); // Get git diff of staged and unstaged changes diff --git a/apps/ui/src/components/views/agent-view.tsx b/apps/ui/src/components/views/agent-view.tsx index f8936cbd..29ceff2b 100644 --- a/apps/ui/src/components/views/agent-view.tsx +++ b/apps/ui/src/components/views/agent-view.tsx @@ -20,14 +20,16 @@ export function AgentView() { const { currentProject } = useAppStore(); const [input, setInput] = useState(''); const [currentTool, setCurrentTool] = useState(null); - // Initialize session manager state based on screen size (hidden on mobile) - const [showSessionManager, setShowSessionManager] = useState(() => { + // Initialize session manager state - starts as true to match SSR + // Then updates on mount based on actual screen size to prevent hydration mismatch + const [showSessionManager, setShowSessionManager] = useState(true); + + // Update session manager visibility based on screen size after mount + useEffect(() => { // Check if we're on a mobile screen (< lg breakpoint = 1024px) - if (typeof window !== 'undefined') { - return window.innerWidth >= 1024; - } - return true; // Default to showing on SSR - }); + const isDesktop = window.innerWidth >= 1024; + setShowSessionManager(isDesktop); + }, []); const [modelSelection, setModelSelection] = useState({ model: 'sonnet' }); // Input ref for auto-focus diff --git a/apps/ui/src/components/views/board-view/components/list-view/list-row.tsx b/apps/ui/src/components/views/board-view/components/list-view/list-row.tsx index f2d57253..817a1e72 100644 --- a/apps/ui/src/components/views/board-view/components/list-view/list-row.tsx +++ b/apps/ui/src/components/views/board-view/components/list-view/list-row.tsx @@ -10,42 +10,6 @@ import type { PipelineConfig } from '@automaker/types'; import { RowActions, type RowActionHandlers } from './row-actions'; import { getColumnWidth, getColumnAlign } from './list-header'; -/** - * Format a date string for display in the table - */ -function formatRelativeDate(dateString: string | undefined): string { - if (!dateString) return '-'; - - const date = new Date(dateString); - const now = new Date(); - const diffMs = now.getTime() - date.getTime(); - const diffDays = Math.floor(diffMs / (1000 * 60 * 60 * 24)); - - if (diffDays === 0) { - // Today - show time - return date.toLocaleTimeString('en-US', { - hour: 'numeric', - minute: '2-digit', - hour12: true, - }); - } - - if (diffDays === 1) { - return 'Yesterday'; - } - - if (diffDays < 7) { - return `${diffDays} days ago`; - } - - // Older - show date - return date.toLocaleDateString('en-US', { - month: 'short', - day: 'numeric', - year: date.getFullYear() !== now.getFullYear() ? 'numeric' : undefined, - }); -} - /** * Get the priority display configuration */ diff --git a/apps/ui/src/components/views/graph-view/graph-canvas.tsx b/apps/ui/src/components/views/graph-view/graph-canvas.tsx index 4c0c279b..88909f32 100644 --- a/apps/ui/src/components/views/graph-view/graph-canvas.tsx +++ b/apps/ui/src/components/views/graph-view/graph-canvas.tsx @@ -258,10 +258,19 @@ function GraphCanvasInner({ let previousWidth = window.innerWidth; let previousHeight = window.innerHeight; + // Track timeout IDs for cleanup + let orientationTimeoutId: ReturnType | null = null; + let resizeTimeoutId: ReturnType | null = null; + const handleOrientationChange = () => { + // Clear any pending timeout + if (orientationTimeoutId) { + clearTimeout(orientationTimeoutId); + } // Small delay to allow the browser to complete the orientation change - setTimeout(() => { + orientationTimeoutId = setTimeout(() => { fitView({ padding: 0.2, duration: 300 }); + orientationTimeoutId = null; }, 100); }; @@ -279,9 +288,14 @@ function GraphCanvasInner({ const isOrientationChange = widthDiff < 100 && heightDiff < 100; if (isOrientationChange) { + // Clear any pending timeout + if (resizeTimeoutId) { + clearTimeout(resizeTimeoutId); + } // Delay fitView to allow browser to complete the layout - setTimeout(() => { + resizeTimeoutId = setTimeout(() => { fitView({ padding: 0.2, duration: 300 }); + resizeTimeoutId = null; }, 150); } @@ -297,6 +311,13 @@ function GraphCanvasInner({ return () => { window.removeEventListener('orientationchange', handleOrientationChange); window.removeEventListener('resize', handleResize); + // Clear any pending timeouts + if (orientationTimeoutId) { + clearTimeout(orientationTimeoutId); + } + if (resizeTimeoutId) { + clearTimeout(resizeTimeoutId); + } }; }, [fitView]); diff --git a/apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx b/apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx index 998f89bc..1b3a6a2e 100644 --- a/apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx +++ b/apps/ui/src/components/views/settings-view/api-keys/claude-usage-section.tsx @@ -82,10 +82,13 @@ export function ClaudeUsageSection() { useEffect(() => { // Initial fetch if authenticated and stale - if (canFetchUsage && isStale) { + // Compute staleness inside effect to avoid re-running when Date.now() changes + const isDataStale = + !claudeUsageLastUpdated || Date.now() - claudeUsageLastUpdated > STALE_THRESHOLD_MS; + if (canFetchUsage && isDataStale) { void fetchUsage(); } - }, [fetchUsage, canFetchUsage, isStale]); + }, [fetchUsage, canFetchUsage, claudeUsageLastUpdated]); useEffect(() => { if (!canFetchUsage) return undefined; diff --git a/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx b/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx index 82d31c4d..a1457be2 100644 --- a/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx +++ b/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx @@ -175,28 +175,29 @@ export function SettingsNavigation({ }: SettingsNavigationProps) { // On mobile, only show when isOpen is true // On desktop (lg+), always show regardless of isOpen - const shouldShow = isOpen; - - if (!shouldShow) { - return null; - } + // The desktop visibility is handled by CSS, but we need to render on mobile only when open return ( <> - {/* Mobile backdrop overlay */} -
+ {/* Mobile backdrop overlay - only shown when isOpen is true on mobile */} + {isOpen && ( +
+ )} {/* Navigation sidebar */}