From fc04ee27948464509ae110fababd25425ed261b1 Mon Sep 17 00:00:00 2001 From: Agent Relay Date: Tue, 13 Jan 2026 03:34:47 +0000 Subject: [PATCH] Fix trajectory viewer race condition with request counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug occurred when clicking trajectories quickly - stale fetch responses could overwrite newer data, showing the wrong trajectory or overview data. Root cause: The previous fix used trajectory ID comparison to reject stale fetches, but this wasn't sufficient because: 1. Polling could start fetches that completed after selection changed 2. Multiple rapid clicks could have overlapping fetches 3. The same trajectory ID check wouldn't catch all race conditions Fix: - Added requestCounterRef to uniquely identify each fetch request - Increment counter in selectTrajectory() to immediately invalidate in-flight fetches when selection changes - Double-check both request ID AND trajectory ID before updating state - Added early return if clicking same trajectory (prevents re-fetch) - Removed redundant initial fetchSteps() call (refresh() handles it) - Only update error state if request is still current The request counter provides stronger protection than ID comparison alone because it catches ALL out-of-order responses, not just those for different trajectory IDs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../react-components/hooks/useTrajectory.ts | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/dashboard/react-components/hooks/useTrajectory.ts b/src/dashboard/react-components/hooks/useTrajectory.ts index 45bb223af..ffdeac59a 100644 --- a/src/dashboard/react-components/hooks/useTrajectory.ts +++ b/src/dashboard/react-components/hooks/useTrajectory.ts @@ -68,6 +68,9 @@ export function useTrajectory(options: UseTrajectoryOptions = {}): UseTrajectory const hasInitializedRef = useRef(false); // Track the latest selection to prevent stale fetches from overwriting data const latestSelectionRef = useRef(selectedTrajectoryId); + // Request counter to ensure only the most recent fetch updates state + // This is more robust than trajectory ID comparison for handling race conditions + const requestCounterRef = useRef(0); // Fetch trajectory status const fetchStatus = useCallback(async () => { @@ -111,9 +114,12 @@ export function useTrajectory(options: UseTrajectoryOptions = {}): UseTrajectory // Fetch trajectory steps const fetchSteps = useCallback(async () => { + // Increment request counter and capture it for this request + // This ensures only the most recent request updates state + const requestId = ++requestCounterRef.current; + const trajectoryId = selectedTrajectoryId; + try { - // Capture the ID this fetch is for - const trajectoryId = selectedTrajectoryId; const basePath = trajectoryId ? `/api/trajectory/steps?trajectoryId=${encodeURIComponent(trajectoryId)}` : '/api/trajectory/steps'; @@ -124,8 +130,12 @@ export function useTrajectory(options: UseTrajectoryOptions = {}): UseTrajectory const response = await fetch(url, { credentials: 'include' }); const data = await response.json(); - // Only update state if this fetch matches the current selection - // This prevents stale fetches from overwriting newer data + // Only update state if this is still the most recent request + // Check both request counter AND trajectory ID for double protection + if (requestId !== requestCounterRef.current) { + console.log('[useTrajectory] Ignoring superseded fetch (request', requestId, 'current', requestCounterRef.current, ')'); + return; + } if (trajectoryId !== latestSelectionRef.current) { console.log('[useTrajectory] Ignoring stale fetch for', trajectoryId, 'current is', latestSelectionRef.current); return; @@ -138,8 +148,11 @@ export function useTrajectory(options: UseTrajectoryOptions = {}): UseTrajectory setError(data.error || 'Failed to fetch trajectory steps'); } } catch (err: any) { - console.error('[useTrajectory] Steps fetch error:', err); - setError(err.message); + // Only update error state if this is still the current request + if (requestId === requestCounterRef.current && trajectoryId === latestSelectionRef.current) { + console.error('[useTrajectory] Steps fetch error:', err); + setError(err.message); + } } }, [apiBaseUrl, selectedTrajectoryId]); @@ -148,15 +161,21 @@ export function useTrajectory(options: UseTrajectoryOptions = {}): UseTrajectory // Normalize empty string to null for consistency const normalizedId = id === '' ? null : id; + // Skip if already selected (prevents unnecessary re-fetches) + if (normalizedId === selectedTrajectoryId) { + return; + } + + // Increment request counter to invalidate any in-flight fetches immediately + // This is crucial - it ensures that even if an old fetch completes after this, + // its request ID won't match and it will be ignored + requestCounterRef.current++; + // Update the ref immediately so in-flight fetches for other trajectories are ignored latestSelectionRef.current = normalizedId; // Clear steps immediately when switching trajectories to prevent showing stale data - // This is the key fix - without this, the old trajectory's steps remain visible - // until the new fetch completes, which looks like "loading wrong trajectory" - if (normalizedId !== selectedTrajectoryId) { - setSteps([]); - } + setSteps([]); // Set loading immediately to avoid flash of empty state before effect runs if (normalizedId !== null) { @@ -188,13 +207,15 @@ export function useTrajectory(options: UseTrajectoryOptions = {}): UseTrajectory }, [refresh]); // Re-fetch steps when selected trajectory changes + // Note: Initial fetch is handled by the refresh() call in the mount effect useEffect(() => { + // Skip the initial render - refresh() handles it if (!hasLoadedInitialStepsRef.current) { hasLoadedInitialStepsRef.current = true; - fetchSteps(); return; } + // For subsequent selection changes, fetch with loading state management let cancelled = false; setIsLoading(true); fetchSteps().finally(() => {