From e3e6c93da0fc8708b60982da5473d52dbbcdbe72 Mon Sep 17 00:00:00 2001 From: Gabriel Gordon-Hall Date: Tue, 8 Jul 2025 09:26:08 +0100 Subject: [PATCH] fix: stop passing stale `task_id` from the frontend (#86) * use selected_attempt.task_id rather than task_id (cherry picked from commit ff31951daa8b52394607ab2aa5fa04ffdc8d32e1) * use selected.task_id when calling get_task_attempt_diff * format * add diffLoading to dependencies in callback * adding diffLoading to dependencies caused an infinite loop; implemented Claude Code suggested alternative --- .../src/components/tasks/TaskDetailsPanel.tsx | 18 ++++++++++---- .../components/tasks/TaskDetailsToolbar.tsx | 22 ++++++++--------- frontend/src/hooks/useTaskDetails.ts | 24 ++++++++++++------- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/frontend/src/components/tasks/TaskDetailsPanel.tsx b/frontend/src/components/tasks/TaskDetailsPanel.tsx index 7f1a6eb1..c7720bbe 100644 --- a/frontend/src/components/tasks/TaskDetailsPanel.tsx +++ b/frontend/src/components/tasks/TaskDetailsPanel.tsx @@ -119,19 +119,28 @@ export function TaskDetailsPanel({ handleSendFollowUp, } = useTaskDetails(task, projectId, isOpen); + // Use ref to track loading state to prevent dependency cycles + const diffLoadingRef = useRef(false); + // Fetch diff when attempt changes const fetchDiff = useCallback(async () => { - if (!projectId || !task?.id || !selectedAttempt?.id) { + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) { setDiff(null); setDiffLoading(false); return; } + // Prevent multiple concurrent requests + if (diffLoadingRef.current) { + return; + } + try { + diffLoadingRef.current = true; setDiffLoading(true); setDiffError(null); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/diff` + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/diff` ); if (response.ok) { @@ -147,9 +156,10 @@ export function TaskDetailsPanel({ } catch (err) { setDiffError('Failed to load diff'); } finally { + diffLoadingRef.current = false; setDiffLoading(false); } - }, [projectId, task?.id, selectedAttempt?.id]); + }, [projectId, selectedAttempt?.id, selectedAttempt?.task_id]); useEffect(() => { if (isOpen) { @@ -463,7 +473,7 @@ export function TaskDetailsPanel({ try { setDeletingFiles((prev) => new Set(prev).add(fileToDelete)); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/delete-file?file_path=${encodeURIComponent( + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/delete-file?file_path=${encodeURIComponent( fileToDelete )}`, { diff --git a/frontend/src/components/tasks/TaskDetailsToolbar.tsx b/frontend/src/components/tasks/TaskDetailsToolbar.tsx index c0290aaa..6219be93 100644 --- a/frontend/src/components/tasks/TaskDetailsToolbar.tsx +++ b/frontend/src/components/tasks/TaskDetailsToolbar.tsx @@ -141,12 +141,12 @@ export function TaskDetailsToolbar({ // Branch status fetching const fetchBranchStatus = useCallback(async () => { - if (!projectId || !task.id || !selectedAttempt?.id) return; + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) return; try { setBranchStatusLoading(true); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/branch-status` + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/branch-status` ); if (response.ok) { @@ -164,7 +164,7 @@ export function TaskDetailsToolbar({ } finally { setBranchStatusLoading(false); } - }, [projectId, task.id, selectedAttempt?.id]); + }, [projectId, selectedAttempt?.id, selectedAttempt?.task_id]); // Fetch branch status when selected attempt changes useEffect(() => { @@ -175,19 +175,19 @@ export function TaskDetailsToolbar({ // Git operations const handleMergeClick = async () => { - if (!projectId || !task.id || !selectedAttempt?.id) return; + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) return; // Directly perform merge without checking branch status await performMerge(); }; const performMerge = async () => { - if (!projectId || !task.id || !selectedAttempt?.id) return; + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) return; try { setMerging(true); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/merge`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/merge`, { method: 'POST', } @@ -212,12 +212,12 @@ export function TaskDetailsToolbar({ }; const handleRebaseClick = async () => { - if (!projectId || !task.id || !selectedAttempt?.id) return; + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) return; try { setRebasing(true); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/rebase`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/rebase`, { method: 'POST', } @@ -242,7 +242,7 @@ export function TaskDetailsToolbar({ }; const handleCreatePRClick = async () => { - if (!projectId || !task.id || !selectedAttempt?.id) return; + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) return; // If PR already exists, open it if (selectedAttempt.pr_url) { @@ -258,12 +258,12 @@ export function TaskDetailsToolbar({ }; const handleConfirmCreatePR = async () => { - if (!projectId || !task.id || !selectedAttempt?.id) return; + if (!projectId || !selectedAttempt?.id || !selectedAttempt?.task_id) return; try { setCreatingPR(true); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/create-pr`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/create-pr`, { method: 'POST', headers: { diff --git a/frontend/src/hooks/useTaskDetails.ts b/frontend/src/hooks/useTaskDetails.ts index 037fb530..f01da9dd 100644 --- a/frontend/src/hooks/useTaskDetails.ts +++ b/frontend/src/hooks/useTaskDetails.ts @@ -128,13 +128,17 @@ export function useTaskDetails( async (attemptId: string) => { if (!task) return; + // Find the attempt to get the task_id + const attempt = taskAttempts.find((a) => a.id === attemptId); + const taskId = attempt?.task_id || task.id; + try { const [activitiesResponse, processesResponse] = await Promise.all([ makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${attemptId}/activities` + `/api/projects/${projectId}/tasks/${taskId}/attempts/${attemptId}/activities` ), makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${attemptId}/execution-processes` + `/api/projects/${projectId}/tasks/${taskId}/attempts/${attemptId}/execution-processes` ), ]); @@ -222,9 +226,13 @@ export function useTaskDetails( async (attemptId: string) => { if (!task) return; + // Find the attempt to get the task_id + const attempt = taskAttempts.find((a) => a.id === attemptId); + const taskId = attempt?.task_id || task.id; + try { const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${attemptId}` + `/api/projects/${projectId}/tasks/${taskId}/attempts/${attemptId}` ); if (response.ok) { @@ -414,7 +422,7 @@ export function useTaskDetails( try { setIsStopping(true); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/stop`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/stop`, { method: 'POST', headers: { @@ -443,7 +451,7 @@ export function useTaskDetails( try { const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/start-dev-server`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/start-dev-server`, { method: 'POST', headers: { @@ -477,7 +485,7 @@ export function useTaskDetails( try { const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/execution-processes/${runningDevServer.id}/stop`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/execution-processes/${runningDevServer.id}/stop`, { method: 'POST', headers: { @@ -503,7 +511,7 @@ export function useTaskDetails( try { const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/open-editor`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/open-editor`, { method: 'POST', headers: { @@ -529,7 +537,7 @@ export function useTaskDetails( setIsSendingFollowUp(true); setFollowUpError(null); const response = await makeRequest( - `/api/projects/${projectId}/tasks/${task.id}/attempts/${selectedAttempt.id}/follow-up`, + `/api/projects/${projectId}/tasks/${selectedAttempt.task_id}/attempts/${selectedAttempt.id}/follow-up`, { method: 'POST', headers: {