From 029c5ca855ed7037cd6fda1051fd9747f940499b Mon Sep 17 00:00:00 2001 From: Shirone Date: Mon, 12 Jan 2026 22:03:14 +0100 Subject: [PATCH] fix: adress pr comments --- .../routes/generate-commit-message.ts | 38 ++++++- apps/ui/src/components/views/agent-view.tsx | 20 +++- apps/ui/src/components/views/board-view.tsx | 4 +- .../components/list-view/list-row.tsx | 30 +++++- .../components/list-view/list-view.tsx | 5 +- .../dialogs/commit-worktree-dialog.tsx | 3 +- .../views/board-view/kanban-board.tsx | 2 +- .../views/board-view/mobile-usage-bar.tsx | 11 +- .../api-keys/claude-usage-section.tsx | 100 +++++++++--------- apps/ui/src/hooks/use-media-query.ts | 14 ++- 10 files changed, 153 insertions(+), 74 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 eb959166..a450659f 100644 --- a/apps/server/src/routes/worktree/routes/generate-commit-message.ts +++ b/apps/server/src/routes/worktree/routes/generate-commit-message.ts @@ -22,6 +22,34 @@ import { getErrorMessage, logError } from '../common.js'; const logger = createLogger('GenerateCommitMessage'); const execAsync = promisify(exec); +/** Timeout for AI provider calls in milliseconds (30 seconds) */ +const AI_TIMEOUT_MS = 30_000; + +/** + * Wraps an async generator with a timeout. + * If the generator takes longer than the timeout, it throws an error. + */ +async function* withTimeout( + generator: AsyncIterable, + timeoutMs: number +): AsyncGenerator { + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => reject(new Error(`AI provider timed out after ${timeoutMs}ms`)), timeoutMs); + }); + + const iterator = generator[Symbol.asyncIterator](); + let done = false; + + while (!done) { + const result = await Promise.race([iterator.next(), timeoutPromise]); + if (result.done) { + done = true; + } else { + yield result.value; + } + } +} + /** * Get the effective system prompt for commit message generation. * Uses custom prompt from settings if enabled, otherwise falls back to default. @@ -180,14 +208,17 @@ export function createGenerateCommitMessageHandler( const cursorPrompt = `${systemPrompt}\n\n${userPrompt}`; let responseText = ''; - for await (const msg of provider.executeQuery({ + const cursorStream = provider.executeQuery({ prompt: cursorPrompt, model: bareModel, cwd: worktreePath, maxTurns: 1, allowedTools: [], readOnly: true, - })) { + }); + + // Wrap with timeout to prevent indefinite hangs + for await (const msg of withTimeout(cursorStream, AI_TIMEOUT_MS)) { if (msg.type === 'assistant' && msg.message?.content) { for (const block of msg.message.content) { if (block.type === 'text' && block.text) { @@ -211,7 +242,8 @@ export function createGenerateCommitMessageHandler( }, }); - message = await extractTextFromStream(stream); + // Wrap with timeout to prevent indefinite hangs + message = await extractTextFromStream(withTimeout(stream, AI_TIMEOUT_MS)); } if (!message || message.trim().length === 0) { diff --git a/apps/ui/src/components/views/agent-view.tsx b/apps/ui/src/components/views/agent-view.tsx index 29ceff2b..5d877471 100644 --- a/apps/ui/src/components/views/agent-view.tsx +++ b/apps/ui/src/components/views/agent-view.tsx @@ -16,6 +16,9 @@ import { import { NoProjectState, AgentHeader, ChatArea } from './agent-view/components'; import { AgentInputArea } from './agent-view/input-area'; +/** Tailwind lg breakpoint in pixels */ +const LG_BREAKPOINT = 1024; + export function AgentView() { const { currentProject } = useAppStore(); const [input, setInput] = useState(''); @@ -24,12 +27,21 @@ export function AgentView() { // 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 + // Update session manager visibility based on screen size after mount and on resize useEffect(() => { - // Check if we're on a mobile screen (< lg breakpoint = 1024px) - const isDesktop = window.innerWidth >= 1024; - setShowSessionManager(isDesktop); + const updateVisibility = () => { + const isDesktop = window.innerWidth >= LG_BREAKPOINT; + setShowSessionManager(isDesktop); + }; + + // Set initial value + updateVisibility(); + + // Listen for resize events + window.addEventListener('resize', updateVisibility); + return () => window.removeEventListener('resize', updateVisibility); }, []); + const [modelSelection, setModelSelection] = useState({ model: 'sonnet' }); // Input ref for auto-focus diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index 479bdf78..6eef8bdf 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -1320,7 +1320,7 @@ export function BoardView() { handleViewOutput(feature); } }} - className="transition-opacity duration-250" + className="transition-opacity duration-200" /> ) : ( setShowPlanDialog(true)} - className="transition-opacity duration-250" + className="transition-opacity duration-200" /> )} 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 8d353dd5..f3877906 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 @@ -1,7 +1,7 @@ // TODO: Remove @ts-nocheck after fixing BaseFeature's index signature issue // The `[key: string]: unknown` in BaseFeature causes property access type errors // @ts-nocheck -import { memo, useCallback, useMemo } from 'react'; +import { memo, useCallback, useState, useEffect } from 'react'; import { cn } from '@/lib/utils'; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip'; import { AlertCircle, Lock, Hand, Sparkles, FileText } from 'lucide-react'; @@ -49,14 +49,34 @@ const IndicatorBadges = memo(function IndicatorBadges({ feature.skipTests && !feature.error && feature.status === 'backlog'; const hasPlan = feature.planSpec?.content; - // Check if just finished (within 2 minutes) - const isJustFinished = useMemo(() => { + // Check if just finished (within 2 minutes) - uses timer to auto-expire + const [isJustFinished, setIsJustFinished] = useState(false); + + useEffect(() => { if (!feature.justFinishedAt || feature.status !== 'waiting_approval' || feature.error) { - return false; + setIsJustFinished(false); + return; } + const finishedTime = new Date(feature.justFinishedAt).getTime(); const twoMinutes = 2 * 60 * 1000; - return Date.now() - finishedTime < twoMinutes; + const elapsed = Date.now() - finishedTime; + + if (elapsed >= twoMinutes) { + setIsJustFinished(false); + return; + } + + // Set as just finished + setIsJustFinished(true); + + // Set a timeout to clear the "just finished" state when 2 minutes have passed + const remainingTime = twoMinutes - elapsed; + const timeoutId = setTimeout(() => { + setIsJustFinished(false); + }, remainingTime); + + return () => clearTimeout(timeoutId); }, [feature.justFinishedAt, feature.status, feature.error]); const badges: Array<{ diff --git a/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx b/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx index 3dc15829..ae4f2f52 100644 --- a/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx +++ b/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx @@ -12,6 +12,9 @@ import { getStatusLabel, getStatusOrder } from './status-badge'; import { getColumnsWithPipeline } from '../../constants'; import type { SortConfig, SortColumn } from '../../hooks/use-list-view-state'; +/** Empty set constant to avoid creating new instances on each render */ +const EMPTY_SET = new Set(); + /** * Status group configuration for the list view */ @@ -184,7 +187,7 @@ export const ListView = memo(function ListView({ pipelineConfig = null, onAddFeature, isSelectionMode = false, - selectedFeatureIds = new Set(), + selectedFeatureIds = EMPTY_SET, onToggleFeatureSelection, onRowClick, className, diff --git a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx index 2c6bdf95..84b1a8fc 100644 --- a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx @@ -80,7 +80,8 @@ export function CommitWorktreeDialog({ }; const handleKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'Enter' && e.metaKey && !isLoading && message.trim()) { + // Prevent commit while loading or while AI is generating a message + if (e.key === 'Enter' && e.metaKey && !isLoading && !isGenerating && message.trim()) { handleCommit(); } }; diff --git a/apps/ui/src/components/views/board-view/kanban-board.tsx b/apps/ui/src/components/views/board-view/kanban-board.tsx index 2b56139f..54341ad9 100644 --- a/apps/ui/src/components/views/board-view/kanban-board.tsx +++ b/apps/ui/src/components/views/board-view/kanban-board.tsx @@ -114,7 +114,7 @@ export function KanbanBoard({
; + icon: ComponentType<{ className?: string }>; label: string; isLoading: boolean; onRefresh: () => void; - children: React.ReactNode; + children: ReactNode; }) { return (
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 1b3a6a2e..11912ec4 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 @@ -26,6 +26,58 @@ const USAGE_COLOR_CRITICAL = 'bg-red-500'; const USAGE_COLOR_WARNING = 'bg-amber-500'; const USAGE_COLOR_OK = 'bg-indigo-500'; +/** + * Get the appropriate color class for a usage percentage + */ +function getUsageColor(percentage: number): string { + if (percentage >= WARNING_THRESHOLD) { + return USAGE_COLOR_CRITICAL; + } + if (percentage >= CAUTION_THRESHOLD) { + return USAGE_COLOR_WARNING; + } + return USAGE_COLOR_OK; +} + +/** + * Individual usage card displaying a usage metric with progress bar + */ +function UsageCard({ + title, + subtitle, + percentage, + resetText, +}: { + title: string; + subtitle: string; + percentage: number; + resetText?: string; +}) { + const safePercentage = Math.min(Math.max(percentage, 0), MAX_PERCENTAGE); + + return ( +
+
+
+

{title}

+

{subtitle}

+
+ {Math.round(safePercentage)}% +
+
+
+
+ {resetText &&

{resetText}

} +
+ ); +} + export function ClaudeUsageSection() { const claudeAuthStatus = useSetupStore((state) => state.claudeAuthStatus); const { claudeUsage, claudeUsageLastUpdated, setClaudeUsage } = useAppStore(); @@ -100,54 +152,6 @@ export function ClaudeUsageSection() { return () => clearInterval(intervalId); }, [fetchUsage, canFetchUsage]); - const getUsageColor = (percentage: number) => { - if (percentage >= WARNING_THRESHOLD) { - return USAGE_COLOR_CRITICAL; - } - if (percentage >= CAUTION_THRESHOLD) { - return USAGE_COLOR_WARNING; - } - return USAGE_COLOR_OK; - }; - - const UsageCard = ({ - title, - subtitle, - percentage, - resetText, - }: { - title: string; - subtitle: string; - percentage: number; - resetText?: string; - }) => { - const safePercentage = Math.min(Math.max(percentage, 0), MAX_PERCENTAGE); - - return ( -
-
-
-

{title}

-

{subtitle}

-
- - {Math.round(safePercentage)}% - -
-
-
-
- {resetText &&

{resetText}

} -
- ); - }; - return (
{ if (typeof window === 'undefined') return; @@ -19,8 +22,13 @@ export function useMediaQuery(query: string): boolean { setMatches(e.matches); }; - // Set initial value - setMatches(mediaQuery.matches); + // Only sync state when query changes after initial mount + // (initial mount already has correct value from useState initializer) + if (isInitialMount.current) { + isInitialMount.current = false; + } else { + setMatches(mediaQuery.matches); + } // Listen for changes mediaQuery.addEventListener('change', handleChange);