diff --git a/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts b/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts index 432c5f36..1bfd9228 100644 --- a/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts +++ b/apps/frontend/src/__tests__/integration/ipc-bridge.test.ts @@ -148,7 +148,8 @@ describe('IPC Bridge Integration', () => { const submitReview = electronAPI['submitReview'] as ( id: string, approved: boolean, - feedback?: string + feedback?: string, + images?: unknown[] ) => Promise; await submitReview('task-id', false, 'Needs more work'); @@ -156,7 +157,8 @@ describe('IPC Bridge Integration', () => { 'task:review', 'task-id', false, - 'Needs more work' + 'Needs more work', + undefined ); }); }); diff --git a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts index 4f68e10e..c417b2a3 100644 --- a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts @@ -1,8 +1,8 @@ import { ipcMain, BrowserWindow } from 'electron'; import { IPC_CHANNELS, AUTO_BUILD_PATHS, getSpecsDir } from '../../../shared/constants'; -import type { IPCResult, TaskStartOptions, TaskStatus } from '../../../shared/types'; +import type { IPCResult, TaskStartOptions, TaskStatus, ImageAttachment } from '../../../shared/types'; import path from 'path'; -import { existsSync, readFileSync, writeFileSync, renameSync, unlinkSync } from 'fs'; +import { existsSync, readFileSync, writeFileSync, renameSync, unlinkSync, mkdirSync } from 'fs'; import { spawnSync, execFileSync } from 'child_process'; import { getToolPath } from '../../cli-tool-manager'; import { AgentManager } from '../../agent'; @@ -318,7 +318,8 @@ export function registerTaskExecutionHandlers( _, taskId: string, approved: boolean, - feedback?: string + feedback?: string, + images?: ImageAttachment[] ): Promise => { // Find task and project const { task, project } = findTaskAndProject(taskId); @@ -407,10 +408,65 @@ export function registerTaskExecutionHandlers( console.warn('[TASK_REVIEW] Writing QA fix request to:', fixRequestPath); console.warn('[TASK_REVIEW] hasWorktree:', hasWorktree, 'worktreePath:', worktreePath); + // Process images if provided + let imageReferences = ''; + if (images && images.length > 0) { + const imagesDir = path.join(targetSpecDir, 'feedback_images'); + try { + if (!existsSync(imagesDir)) { + mkdirSync(imagesDir, { recursive: true }); + } + const savedImages: string[] = []; + for (const image of images) { + try { + if (!image.data) { + console.warn('[TASK_REVIEW] Skipping image with no data:', image.filename); + continue; + } + // Server-side MIME type validation (defense in depth - frontend also validates) + // Reject missing mimeType to prevent bypass attacks + const ALLOWED_MIME_TYPES = ['image/png', 'image/jpeg', 'image/jpg', 'image/gif', 'image/webp', 'image/svg+xml']; + if (!image.mimeType || !ALLOWED_MIME_TYPES.includes(image.mimeType)) { + console.warn('[TASK_REVIEW] Skipping image with missing or disallowed MIME type:', image.mimeType); + continue; + } + // Sanitize filename to prevent path traversal attacks + const sanitizedFilename = path.basename(image.filename); + if (!sanitizedFilename || sanitizedFilename === '.' || sanitizedFilename === '..') { + console.warn('[TASK_REVIEW] Skipping image with invalid filename:', image.filename); + continue; + } + // Remove data URL prefix if present (e.g., "data:image/png;base64," or "data:image/svg+xml;base64,") + const base64Data = image.data.replace(/^data:image\/[^;]+;base64,/, ''); + const imageBuffer = Buffer.from(base64Data, 'base64'); + const imagePath = path.join(imagesDir, sanitizedFilename); + // Verify the resolved path is within the images directory (defense in depth) + const resolvedPath = path.resolve(imagePath); + const resolvedImagesDir = path.resolve(imagesDir); + if (!resolvedPath.startsWith(resolvedImagesDir + path.sep)) { + console.warn('[TASK_REVIEW] Skipping image with path outside target directory:', image.filename); + continue; + } + writeFileSync(imagePath, imageBuffer); + savedImages.push(`feedback_images/${sanitizedFilename}`); + console.log('[TASK_REVIEW] Saved image:', sanitizedFilename); + } catch (imgError) { + console.error('[TASK_REVIEW] Failed to save image:', image.filename, imgError); + } + } + if (savedImages.length > 0) { + imageReferences = '\n\n## Reference Images\n\n' + + savedImages.map(imgPath => `![Feedback Image](${imgPath})`).join('\n\n'); + } + } catch (dirError) { + console.error('[TASK_REVIEW] Failed to create images directory:', dirError); + } + } + try { writeFileSync( fixRequestPath, - `# QA Fix Request\n\nStatus: REJECTED\n\n## Feedback\n\n${feedback || 'No feedback provided'}\n\nCreated at: ${new Date().toISOString()}\n` + `# QA Fix Request\n\nStatus: REJECTED\n\n## Feedback\n\n${feedback || 'No feedback provided'}${imageReferences}\n\nCreated at: ${new Date().toISOString()}\n` ); } catch (error) { console.error('[TASK_REVIEW] Failed to write QA fix request:', error); diff --git a/apps/frontend/src/preload/api/task-api.ts b/apps/frontend/src/preload/api/task-api.ts index 0fbe408d..417b73f4 100644 --- a/apps/frontend/src/preload/api/task-api.ts +++ b/apps/frontend/src/preload/api/task-api.ts @@ -13,7 +13,8 @@ import type { SupportedIDE, SupportedTerminal, WorktreeCreatePROptions, - WorktreeCreatePRResult + WorktreeCreatePRResult, + ImageAttachment } from '../../shared/types'; export interface TaskAPI { @@ -35,7 +36,8 @@ export interface TaskAPI { submitReview: ( taskId: string, approved: boolean, - feedback?: string + feedback?: string, + images?: ImageAttachment[] ) => Promise; updateTaskStatus: ( taskId: string, @@ -112,9 +114,10 @@ export const createTaskAPI = (): TaskAPI => ({ submitReview: ( taskId: string, approved: boolean, - feedback?: string + feedback?: string, + images?: ImageAttachment[] ): Promise => - ipcRenderer.invoke(IPC_CHANNELS.TASK_REVIEW, taskId, approved, feedback), + ipcRenderer.invoke(IPC_CHANNELS.TASK_REVIEW, taskId, approved, feedback, images), updateTaskStatus: ( taskId: string, diff --git a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx index e51fb2e7..11c6f1de 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx @@ -118,13 +118,15 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, }; const handleReject = async () => { - if (!state.feedback.trim()) { + // Allow submission if there's text feedback OR images attached + if (!state.feedback.trim() && state.feedbackImages.length === 0) { return; } state.setIsSubmitting(true); - await submitReview(task.id, false, state.feedback); + await submitReview(task.id, false, state.feedback, state.feedbackImages); state.setIsSubmitting(false); state.setFeedback(''); + state.setFeedbackImages([]); }; const handleDelete = async () => { @@ -516,6 +518,8 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, showConflictDialog={state.showConflictDialog} onFeedbackChange={state.setFeedback} onReject={handleReject} + images={state.feedbackImages} + onImagesChange={state.setFeedbackImages} onMerge={handleMerge} onDiscard={handleDiscard} onShowDiscardDialog={state.setShowDiscardDialog} diff --git a/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx b/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx index d27a1689..c22085a2 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskReview.tsx @@ -1,4 +1,4 @@ -import type { Task, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, WorktreeCreatePRResult } from '../../../shared/types'; +import type { Task, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, ImageAttachment, WorktreeCreatePRResult } from '../../../shared/types'; import { StagedSuccessMessage, WorkspaceStatus, @@ -33,6 +33,10 @@ interface TaskReviewProps { showConflictDialog: boolean; onFeedbackChange: (value: string) => void; onReject: () => void; + /** Image attachments for visual feedback */ + images?: ImageAttachment[]; + /** Callback when images change */ + onImagesChange?: (images: ImageAttachment[]) => void; onMerge: () => void; onDiscard: () => void; onShowDiscardDialog: (show: boolean) => void; @@ -81,6 +85,8 @@ export function TaskReview({ showConflictDialog, onFeedbackChange, onReject, + images, + onImagesChange, onMerge, onDiscard, onShowDiscardDialog, @@ -157,6 +163,8 @@ export function TaskReview({ isSubmitting={isSubmitting} onFeedbackChange={onFeedbackChange} onReject={onReject} + images={images} + onImagesChange={onImagesChange} /> {/* Discard Confirmation Dialog */} diff --git a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts index ea310269..2a01b3b4 100644 --- a/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts +++ b/apps/frontend/src/renderer/components/task-detail/hooks/useTaskDetail.ts @@ -1,7 +1,7 @@ import { useState, useRef, useEffect, useCallback } from 'react'; import { useProjectStore } from '../../../stores/project-store'; import { checkTaskRunning, isIncompleteHumanReview, getTaskProgress, useTaskStore, loadTasks } from '../../../stores/task-store'; -import type { Task, TaskLogs, TaskLogPhase, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo } from '../../../../shared/types'; +import type { Task, TaskLogs, TaskLogPhase, WorktreeStatus, WorktreeDiff, MergeConflict, MergeStats, GitConflictInfo, ImageAttachment } from '../../../../shared/types'; /** * Validates task subtasks structure to prevent infinite loops during resume. @@ -50,6 +50,7 @@ export interface UseTaskDetailOptions { export function useTaskDetail({ task }: UseTaskDetailOptions) { const [feedback, setFeedback] = useState(''); + const [feedbackImages, setFeedbackImages] = useState([]); const [isSubmitting, setIsSubmitting] = useState(false); const [activeTab, setActiveTab] = useState('overview'); const [isUserScrolledUp, setIsUserScrolledUp] = useState(false); @@ -161,6 +162,11 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { } }, [activeTab]); + // Reset feedback images when task changes to prevent image leakage between tasks + useEffect(() => { + setFeedbackImages([]); + }, [task.id]); + // Load worktree status when task is in human_review useEffect(() => { if (needsReview) { @@ -255,6 +261,26 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { }); }, []); + // Add a feedback image + const addFeedbackImage = useCallback((image: ImageAttachment) => { + setFeedbackImages(prev => [...prev, image]); + }, []); + + // Add multiple feedback images at once + const addFeedbackImages = useCallback((images: ImageAttachment[]) => { + setFeedbackImages(prev => [...prev, ...images]); + }, []); + + // Remove a feedback image by ID + const removeFeedbackImage = useCallback((imageId: string) => { + setFeedbackImages(prev => prev.filter(img => img.id !== imageId)); + }, []); + + // Clear all feedback images + const clearFeedbackImages = useCallback(() => { + setFeedbackImages([]); + }, []); + // Track if we've already loaded preview for this task to prevent infinite loops const hasLoadedPreviewRef = useRef(null); @@ -404,6 +430,7 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { return { // State feedback, + feedbackImages, isSubmitting, activeTab, isUserScrolledUp, @@ -447,6 +474,7 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { // Setters setFeedback, + setFeedbackImages, setIsSubmitting, setActiveTab, setIsUserScrolledUp, @@ -482,6 +510,10 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) { handleLogsScroll, togglePhase, loadMergePreview, + addFeedbackImage, + addFeedbackImages, + removeFeedbackImage, + clearFeedbackImages, handleReviewAgain, reloadPlanForIncompleteTask, }; diff --git a/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx b/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx index 9f07d92f..149f5dbe 100644 --- a/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx +++ b/apps/frontend/src/renderer/components/task-detail/task-review/QAFeedbackSection.tsx @@ -1,54 +1,376 @@ -import { AlertCircle, RotateCcw, Loader2 } from 'lucide-react'; +import { useCallback, useRef, useState, type ClipboardEvent, type DragEvent } from 'react'; +import { useTranslation } from 'react-i18next'; +import { AlertCircle, RotateCcw, Loader2, Image as ImageIcon, X } from 'lucide-react'; import { Button } from '../../ui/button'; import { Textarea } from '../../ui/textarea'; +import { + generateImageId, + blobToBase64, + createThumbnail, + isValidImageMimeType, + resolveFilename +} from '../../ImageUpload'; +import { cn } from '../../../lib/utils'; +import type { ImageAttachment } from '../../../../shared/types'; +import { + MAX_IMAGES_PER_TASK, + ALLOWED_IMAGE_TYPES_DISPLAY +} from '../../../../shared/constants'; interface QAFeedbackSectionProps { feedback: string; isSubmitting: boolean; onFeedbackChange: (value: string) => void; onReject: () => void; + /** Image attachments for visual feedback - optional for backward compatibility */ + images?: ImageAttachment[]; + /** Callback when images change - optional for backward compatibility */ + onImagesChange?: (images: ImageAttachment[]) => void; } /** * Displays the QA feedback section where users can request changes + * Supports image paste and drag-drop for visual feedback */ export function QAFeedbackSection({ feedback, isSubmitting, onFeedbackChange, - onReject + onReject, + images = [], + onImagesChange }: QAFeedbackSectionProps) { + const { t } = useTranslation('tasks'); + + // Feature is enabled when onImagesChange callback is provided + const imageUploadEnabled = !!onImagesChange; + + // Ref for the textarea + const textareaRef = useRef(null); + + // Local state for UI feedback + const [isDragOverTextarea, setIsDragOverTextarea] = useState(false); + const [pasteSuccess, setPasteSuccess] = useState(false); + const [error, setError] = useState(null); + + /** + * Handle paste event for screenshot support + */ + const handlePaste = useCallback(async (e: ClipboardEvent) => { + // Skip image handling if feature is not enabled + if (!onImagesChange) return; + + const clipboardItems = e.clipboardData?.items; + if (!clipboardItems) return; + + // Find image items in clipboard + const imageItems: DataTransferItem[] = []; + for (let i = 0; i < clipboardItems.length; i++) { + const item = clipboardItems[i]; + if (item.type.startsWith('image/')) { + imageItems.push(item); + } + } + + // If no images, allow normal paste behavior + if (imageItems.length === 0) return; + + // Prevent default paste when we have images + e.preventDefault(); + + // Check if we can add more images + const remainingSlots = MAX_IMAGES_PER_TASK - images.length; + if (remainingSlots <= 0) { + setError(t('feedback.maxImagesError', { count: MAX_IMAGES_PER_TASK })); + return; + } + + setError(null); + + // Process image items + const newImages: ImageAttachment[] = []; + const existingFilenames = images.map(img => img.filename); + + for (const item of imageItems.slice(0, remainingSlots)) { + const file = item.getAsFile(); + if (!file) continue; + + // Validate image type + if (!isValidImageMimeType(file.type)) { + setError(t('feedback.invalidTypeError', { types: ALLOWED_IMAGE_TYPES_DISPLAY })); + continue; + } + + try { + const dataUrl = await blobToBase64(file); + const thumbnail = await createThumbnail(dataUrl); + + // Generate filename for pasted images (screenshot-timestamp.ext) + // Map MIME types to proper file extensions (handles svg+xml -> svg, etc.) + const mimeToExtension: Record = { + 'image/svg+xml': 'svg', + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/gif': 'gif', + 'image/webp': 'webp', + }; + const extension = mimeToExtension[file.type] || file.type.split('/')[1] || 'png'; + const baseFilename = `screenshot-${Date.now()}.${extension}`; + const resolvedFilename = resolveFilename(baseFilename, [ + ...existingFilenames, + ...newImages.map(img => img.filename) + ]); + + newImages.push({ + id: generateImageId(), + filename: resolvedFilename, + mimeType: file.type, + size: file.size, + data: dataUrl.split(',')[1], // Store base64 without data URL prefix + thumbnail + }); + } catch (error) { + console.error('[QAFeedbackSection] Failed to process pasted image:', error); + setError(t('feedback.processingError', 'Failed to process pasted image')); + } + } + + if (newImages.length > 0) { + onImagesChange([...images, ...newImages]); + // Show success feedback + setPasteSuccess(true); + setTimeout(() => setPasteSuccess(false), 2000); + } + }, [images, onImagesChange, t]); + + /** + * Handle drag over textarea for image drops + */ + const handleTextareaDragOver = useCallback((e: DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragOverTextarea(true); + }, []); + + /** + * Handle drag leave from textarea + */ + const handleTextareaDragLeave = useCallback((e: DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragOverTextarea(false); + }, []); + + /** + * Handle drop on textarea for images + */ + const handleTextareaDrop = useCallback( + async (e: DragEvent) => { + e.preventDefault(); + e.stopPropagation(); + setIsDragOverTextarea(false); + + // Skip image handling if feature is not enabled + if (!onImagesChange) return; + if (isSubmitting) return; + + const files = e.dataTransfer?.files; + if (!files || files.length === 0) return; + + // Filter for image files + const imageFiles: File[] = []; + for (let i = 0; i < files.length; i++) { + const file = files[i]; + if (file.type.startsWith('image/')) { + imageFiles.push(file); + } + } + + if (imageFiles.length === 0) return; + + // Check if we can add more images + const remainingSlots = MAX_IMAGES_PER_TASK - images.length; + if (remainingSlots <= 0) { + setError(t('feedback.maxImagesError', { count: MAX_IMAGES_PER_TASK })); + return; + } + + setError(null); + + // Process image files + const newImages: ImageAttachment[] = []; + const existingFilenames = images.map(img => img.filename); + + for (const file of imageFiles.slice(0, remainingSlots)) { + // Validate image type + if (!isValidImageMimeType(file.type)) { + setError(t('feedback.invalidTypeError', { types: ALLOWED_IMAGE_TYPES_DISPLAY })); + continue; + } + + try { + const dataUrl = await blobToBase64(file); + const thumbnail = await createThumbnail(dataUrl); + + // Use original filename or generate one with proper extension + // Map MIME types to proper file extensions (handles svg+xml -> svg, etc.) + const mimeToExtension: Record = { + 'image/svg+xml': 'svg', + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/gif': 'gif', + 'image/webp': 'webp', + }; + const extension = mimeToExtension[file.type] || file.type.split('/')[1] || 'png'; + const baseFilename = file.name || `dropped-image-${Date.now()}.${extension}`; + const resolvedFilename = resolveFilename(baseFilename, [ + ...existingFilenames, + ...newImages.map(img => img.filename) + ]); + + newImages.push({ + id: generateImageId(), + filename: resolvedFilename, + mimeType: file.type, + size: file.size, + data: dataUrl.split(',')[1], // Store base64 without data URL prefix + thumbnail + }); + } catch (error) { + console.error('[QAFeedbackSection] Failed to process dropped image:', error); + setError(t('feedback.processingError', 'Failed to process dropped image')); + } + } + + if (newImages.length > 0) { + onImagesChange([...images, ...newImages]); + // Show success feedback + setPasteSuccess(true); + setTimeout(() => setPasteSuccess(false), 2000); + } + }, + [images, isSubmitting, onImagesChange, t] + ); + + /** + * Remove an image from the attachments + */ + const handleRemoveImage = useCallback((imageId: string) => { + if (!onImagesChange) return; + onImagesChange(images.filter(img => img.id !== imageId)); + setError(null); + }, [images, onImagesChange]); + + // Allow submission with either text feedback or images + const canSubmit = feedback.trim() || images.length > 0; + return (

- Request Changes + {t('feedback.requestChanges', 'Request Changes')}

- Found issues? Describe what needs to be fixed and the AI will continue working on it. + {t('feedback.description', 'Found issues? Describe what needs to be fixed and the AI will continue working on it.')}

+ + {/* Textarea with paste/drop support */}