Skip to content

Fixes critical React crash on the Kanban board view #830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gsxdsm merged 2 commits into from
Mar 4, 2026
Merged

Fixes critical React crash on the Kanban board view #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
387fe98
Changes from fix/board-react-crash
gsxdsm Mar 4, 2026
2e0423b
fix: Prevent cascading re-renders and crashes from high-frequency WS …
gsxdsm Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 31 additions & 52 deletions apps/ui/src/components/views/board-view/hooks/use-board-actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @ts-nocheck - feature update logic with partial updates and image/file handling
import { useCallback } from 'react';
import { useQueryClient } from '@tanstack/react-query';
import {
Feature,
FeatureImage,
Expand All @@ -18,7 +17,10 @@ import { useVerifyFeature, useResumeFeature } from '@/hooks/mutations';
import { truncateDescription } from '@/lib/utils';
import { getBlockingDependencies } from '@automaker/dependency-resolver';
import { createLogger } from '@automaker/utils/logger';
import { queryKeys } from '@/lib/query-keys';
import {
markFeatureTransitioning,
unmarkFeatureTransitioning,
} from '@/lib/feature-transition-state';

const logger = createLogger('BoardActions');

Expand Down Expand Up @@ -116,16 +118,13 @@ export function useBoardActions({
currentWorktreeBranch,
stopFeature,
}: UseBoardActionsProps) {
const queryClient = useQueryClient();

// IMPORTANT: Use individual selectors instead of bare useAppStore() to prevent
// subscribing to the entire store. Bare useAppStore() causes the host component
// (BoardView) to re-render on EVERY store change, which cascades through effects
// and triggers React error #185 (maximum update depth exceeded).
const addFeature = useAppStore((s) => s.addFeature);
const updateFeature = useAppStore((s) => s.updateFeature);
const removeFeature = useAppStore((s) => s.removeFeature);
const moveFeature = useAppStore((s) => s.moveFeature);
const worktreesEnabled = useAppStore((s) => s.useWorktrees);
const enableDependencyBlocking = useAppStore((s) => s.enableDependencyBlocking);
const skipVerificationInAutoMode = useAppStore((s) => s.skipVerificationInAutoMode);
Expand Down Expand Up @@ -707,8 +706,7 @@ export function useBoardActions({
try {
const result = await verifyFeatureMutation.mutateAsync(feature.id);
if (result.passes) {
// Immediately move card to verified column (optimistic update)
moveFeature(feature.id, 'verified');
// persistFeatureUpdate handles the optimistic RQ cache update internally
persistFeatureUpdate(feature.id, {
status: 'verified',
justFinishedAt: undefined,
Expand All @@ -725,7 +723,7 @@ export function useBoardActions({
// Error toast is already shown by the mutation's onError handler
}
},
[currentProject, verifyFeatureMutation, moveFeature, persistFeatureUpdate]
[currentProject, verifyFeatureMutation, persistFeatureUpdate]
);

const handleResumeFeature = useCallback(
Expand All @@ -742,7 +740,6 @@ export function useBoardActions({

const handleManualVerify = useCallback(
(feature: Feature) => {
moveFeature(feature.id, 'verified');
persistFeatureUpdate(feature.id, {
status: 'verified',
justFinishedAt: undefined,
Expand All @@ -751,7 +748,7 @@ export function useBoardActions({
description: `Marked as verified: ${truncateDescription(feature.description)}`,
});
},
[moveFeature, persistFeatureUpdate]
[persistFeatureUpdate]
);

const handleMoveBackToInProgress = useCallback(
Expand All @@ -760,13 +757,12 @@ export function useBoardActions({
status: 'in_progress' as const,
startedAt: new Date().toISOString(),
};
updateFeature(feature.id, updates);
persistFeatureUpdate(feature.id, updates);
toast.info('Feature moved back', {
description: `Moved back to In Progress: ${truncateDescription(feature.description)}`,
});
},
[updateFeature, persistFeatureUpdate]
[persistFeatureUpdate]
);

const handleOpenFollowUp = useCallback(
Expand Down Expand Up @@ -885,7 +881,6 @@ export function useBoardActions({
);

if (result.success) {
moveFeature(feature.id, 'verified');
persistFeatureUpdate(feature.id, { status: 'verified' });
toast.success('Feature committed', {
description: `Committed and verified: ${truncateDescription(feature.description)}`,
Expand All @@ -907,7 +902,7 @@ export function useBoardActions({
await loadFeatures();
}
},
[currentProject, moveFeature, persistFeatureUpdate, loadFeatures, onWorktreeCreated]
[currentProject, persistFeatureUpdate, loadFeatures, onWorktreeCreated]
);

const handleMergeFeature = useCallback(
Expand Down Expand Up @@ -951,17 +946,12 @@ export function useBoardActions({

const handleCompleteFeature = useCallback(
(feature: Feature) => {
const updates = {
status: 'completed' as const,
};
updateFeature(feature.id, updates);
persistFeatureUpdate(feature.id, updates);

persistFeatureUpdate(feature.id, { status: 'completed' as const });
toast.success('Feature completed', {
description: `Archived: ${truncateDescription(feature.description)}`,
});
},
[updateFeature, persistFeatureUpdate]
[persistFeatureUpdate]
);

const handleUnarchiveFeature = useCallback(
Expand All @@ -978,11 +968,7 @@ export function useBoardActions({
(projectPath ? isPrimaryWorktreeBranch(projectPath, currentWorktreeBranch) : true)
: featureBranch === currentWorktreeBranch;

const updates: Partial<Feature> = {
status: 'verified' as const,
};
updateFeature(feature.id, updates);
persistFeatureUpdate(feature.id, updates);
persistFeatureUpdate(feature.id, { status: 'verified' as const });

if (willBeVisibleOnCurrentView) {
toast.success('Feature restored', {
Expand All @@ -994,13 +980,7 @@ export function useBoardActions({
});
}
},
[
updateFeature,
persistFeatureUpdate,
currentWorktreeBranch,
projectPath,
isPrimaryWorktreeBranch,
]
[persistFeatureUpdate, currentWorktreeBranch, projectPath, isPrimaryWorktreeBranch]
);

const handleViewOutput = useCallback(
Expand Down Expand Up @@ -1031,6 +1011,13 @@ export function useBoardActions({

const handleForceStopFeature = useCallback(
async (feature: Feature) => {
// Mark this feature as transitioning so WebSocket-driven query invalidation
// (useAutoModeQueryInvalidation) skips redundant cache invalidations while
// persistFeatureUpdate is handling the optimistic update. Without this guard,
// auto_mode_error / auto_mode_stopped WS events race with the optimistic
// update and cause cache flip-flops that cascade through useBoardColumnFeatures,
// triggering React error #185 on mobile.
markFeatureTransitioning(feature.id);
Comment on lines +1014 to +1020
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make transition guarding re-entrant for repeated stop requests.

markFeatureTransitioning(feature.id) + delayed unmark is overlap-unsafe: repeated stop actions for the same feature can clear the flag while another transition is still active, re-enabling WS invalidations too early.

Ref-counted guard approach 
 const MAX_DUPLICATES = 50;
+const stopTransitionRefCounts = new Map<string, number>();

 ...
-      markFeatureTransitioning(feature.id);
+      const count = stopTransitionRefCounts.get(feature.id) ?? 0;
+      stopTransitionRefCounts.set(feature.id, count + 1);
+      if (count === 0) {
+        markFeatureTransitioning(feature.id);
+      }

 ...
-        setTimeout(() => unmarkFeatureTransitioning(feature.id), 500);
+        setTimeout(() => {
+          const next = (stopTransitionRefCounts.get(feature.id) ?? 1) - 1;
+          if (next <= 0) {
+            stopTransitionRefCounts.delete(feature.id);
+            unmarkFeatureTransitioning(feature.id);
+          } else {
+            stopTransitionRefCounts.set(feature.id, next);
+          }
+        }, 500);

Also applies to: 1059-1065

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/ui/src/components/views/board-view/hooks/use-board-actions.ts` around
lines 1014 - 1020, The transition guard is not re-entrant: replace the boolean
markFeatureTransitioning/unmarkFeatureTransitioning behavior with a ref-counted
guard so repeated stop requests for the same feature are safe; update the
implementations used in use-board-actions (markFeatureTransitioning and
unmarkFeatureTransitioning) to increment a counter for feature.id on
markFeatureTransitioning and decrement on unmarkFeatureTransitioning (only
removing the transitioning state when the counter drops to zero), and ensure any
delayed/unmount cleanup paths call the decrementing unmark so overlapping
delayed unmarks don’t prematurely clear the guard.

try {
await stopFeature(feature.id);

Expand All @@ -1048,25 +1035,11 @@ export function useBoardActions({
removeRunningTaskFromAllWorktrees(currentProject.id, feature.id);
}

// Optimistically update the React Query features cache so the board
// moves the card immediately. Without this, the card stays in
// "in_progress" until the next poll cycle (30s) because the async
// refetch races with the persistFeatureUpdate write.
if (currentProject) {
queryClient.setQueryData(
queryKeys.features.all(currentProject.path),
(oldFeatures: Feature[] | undefined) => {
if (!oldFeatures) return oldFeatures;
return oldFeatures.map((f) =>
f.id === feature.id ? { ...f, status: targetStatus } : f
);
}
);
}

if (targetStatus !== feature.status) {
moveFeature(feature.id, targetStatus);
// Must await to ensure file is written before user can restart
// persistFeatureUpdate handles the optimistic RQ cache update, the
// Zustand store update (on server response), and the final cache
// invalidation internally — no need for separate queryClient.setQueryData
// or moveFeature calls which would cause redundant re-renders.
await persistFeatureUpdate(feature.id, { status: targetStatus });
}

Expand All @@ -1083,9 +1056,15 @@ export function useBoardActions({
toast.error('Failed to stop agent', {
description: error instanceof Error ? error.message : 'An error occurred',
});
} finally {
// Delay unmarking so the refetch triggered by persistFeatureUpdate's
// invalidateQueries() has time to settle before WS-driven invalidations
// are allowed through again. Without this, a WS event arriving during
// the refetch window would trigger a conflicting invalidation.
setTimeout(() => unmarkFeatureTransitioning(feature.id), 500);
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While using a setTimeout here is a pragmatic solution for the race condition, the magic number 500 can be hard to maintain. Consider defining it as a named constant at the top of the file (e.g., UNMARK_TRANSITION_DELAY_MS) to clarify its purpose and make it easier to adjust globally if needed.

}
},
[stopFeature, moveFeature, persistFeatureUpdate, currentProject, queryClient]
[stopFeature, persistFeatureUpdate, currentProject]
);

const handleStartNextFeatures = useCallback(async () => {
Expand Down
21 changes: 10 additions & 11 deletions apps/ui/src/components/views/board-view/hooks/use-board-column-features.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-nocheck - column filtering logic with dependency resolution and status mapping
import { useMemo, useCallback, useEffect, useRef } from 'react';
import { useMemo, useCallback, useEffect } from 'react';
import { Feature, useAppStore } from '@/store/app-store';
import {
createFeatureMap,
Expand Down Expand Up @@ -177,9 +177,6 @@ export function useBoardColumnFeatures({
(state) => state.clearRecentlyCompletedFeatures
);

// Track previous feature IDs to detect when features list has been refreshed
const prevFeatureIdsRef = useRef<Set<string>>(new Set());

// Clear recently completed features when the cache refreshes with updated statuses.
//
// RACE CONDITION SCENARIO THIS PREVENTS:
Expand All @@ -193,22 +190,24 @@ export function useBoardColumnFeatures({
//
// When the refetch completes with fresh data (status='verified'/'completed'),
// this effect clears the recentlyCompletedFeatures set since it's no longer needed.
// Clear recently completed features when the cache refreshes with updated statuses.
// IMPORTANT: Only depend on `features` (not `recentlyCompletedFeatures`) to avoid a
// re-trigger loop where clearing the set creates a new reference that re-fires this effect.
// Read recentlyCompletedFeatures from the store directly to get the latest value without
// subscribing to it as a dependency.
useEffect(() => {
const currentIds = new Set(features.map((f) => f.id));
const currentRecentlyCompleted = useAppStore.getState().recentlyCompletedFeatures;
if (currentRecentlyCompleted.size === 0) return;

// Check if any recently completed features now have terminal statuses in the new data
// If so, we can clear the tracking since the cache is now fresh
const hasUpdatedStatus = Array.from(recentlyCompletedFeatures).some((featureId) => {
const hasUpdatedStatus = Array.from(currentRecentlyCompleted).some((featureId) => {
const feature = features.find((f) => f.id === featureId);
return feature && (feature.status === 'verified' || feature.status === 'completed');
});

if (hasUpdatedStatus) {
clearRecentlyCompletedFeatures();
}

prevFeatureIdsRef.current = currentIds;
}, [features, recentlyCompletedFeatures, clearRecentlyCompletedFeatures]);
}, [features, clearRecentlyCompletedFeatures]);
Comment on lines 198 to +210
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This useEffect is well-refactored to avoid dependency loops. For a minor performance improvement, you can optimize the lookup inside the .some() call. Currently, it uses features.find(), which has O(N) complexity, making the total complexity O(M * N) where M is the size of recentlyCompletedFeatures and N is the number of features. By building a Map of features first, you can achieve an O(1) lookup, improving the overall performance of this effect, which is in line with the goals of this PR.

Suggested change
useEffect(() => {
const currentIds = new Set(features.map((f) => f.id));
const currentRecentlyCompleted = useAppStore.getState().recentlyCompletedFeatures;
if (currentRecentlyCompleted.size === 0) return;
// Check if any recently completed features now have terminal statuses in the new data
// If so, we can clear the tracking since the cache is now fresh
const hasUpdatedStatus = Array.from(recentlyCompletedFeatures).some((featureId) => {
const hasUpdatedStatus = Array.from(currentRecentlyCompleted).some((featureId) => {
const feature = features.find((f) => f.id === featureId);
return feature && (feature.status === 'verified' || feature.status === 'completed');
});
if (hasUpdatedStatus) {
clearRecentlyCompletedFeatures();
}
prevFeatureIdsRef.current = currentIds;
}, [features, recentlyCompletedFeatures, clearRecentlyCompletedFeatures]);
}, [features, clearRecentlyCompletedFeatures]);
useEffect(() => {
const currentRecentlyCompleted = useAppStore.getState().recentlyCompletedFeatures;
if (currentRecentlyCompleted.size === 0 || features.length === 0) return;
const featureMap = new Map(features.map((f) => [f.id, f]));
const hasUpdatedStatus = Array.from(currentRecentlyCompleted).some((featureId) => {
const feature = featureMap.get(featureId);
return feature && (feature.status === 'verified' || feature.status === 'completed');
});
if (hasUpdatedStatus) {
clearRecentlyCompletedFeatures();
}
}, [features, clearRecentlyCompletedFeatures]);


// Memoize column features to prevent unnecessary re-renders
const columnFeaturesMap = useMemo(() => {
Expand Down
14 changes: 2 additions & 12 deletions apps/ui/src/components/views/board-view/hooks/use-board-drag-drop.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export function useBoardDragDrop({
// subscribing to the entire store. Bare useAppStore() causes the host component
// (BoardView) to re-render on EVERY store change, which cascades through effects
// and triggers React error #185 (maximum update depth exceeded).
const moveFeature = useAppStore((s) => s.moveFeature);
const updateFeature = useAppStore((s) => s.updateFeature);

// Note: getOrCreateWorktreeForFeature removed - worktrees are now created server-side
Expand Down Expand Up @@ -207,23 +206,22 @@ export function useBoardDragDrop({
if (targetStatus === draggedFeature.status) return;

// Handle different drag scenarios
// Note: Worktrees are created server-side at execution time based on feature.branchName
// Note: persistFeatureUpdate handles optimistic RQ cache update internally,
// so no separate moveFeature() call is needed.
if (draggedFeature.status === 'backlog' || draggedFeature.status === 'merge_conflict') {
// From backlog
if (targetStatus === 'in_progress') {
// Use helper function to handle concurrency check and start implementation
// Server will derive workDir from feature.branchName
await handleStartImplementation(draggedFeature);
} else {
moveFeature(featureId, targetStatus);
persistFeatureUpdate(featureId, { status: targetStatus });
}
} else if (draggedFeature.status === 'waiting_approval') {
// waiting_approval features can be dragged to verified for manual verification
// NOTE: This check must come BEFORE skipTests check because waiting_approval
// features often have skipTests=true, and we want status-based handling first
if (targetStatus === 'verified') {
moveFeature(featureId, 'verified');
// Clear justFinishedAt timestamp when manually verifying via drag
persistFeatureUpdate(featureId, {
status: 'verified',
Expand All @@ -237,7 +235,6 @@ export function useBoardDragDrop({
});
} else if (targetStatus === 'backlog') {
// Allow moving waiting_approval cards back to backlog
moveFeature(featureId, 'backlog');
// Clear justFinishedAt timestamp when moving back to backlog
persistFeatureUpdate(featureId, {
status: 'backlog',
Expand Down Expand Up @@ -269,7 +266,6 @@ export function useBoardDragDrop({
});
}
}
moveFeature(featureId, 'backlog');
persistFeatureUpdate(featureId, { status: 'backlog' });
toast.info(
isRunningTask
Expand All @@ -291,7 +287,6 @@ export function useBoardDragDrop({
return;
} else if (targetStatus === 'verified' && draggedFeature.skipTests) {
// Manual verify via drag (only for skipTests features)
moveFeature(featureId, 'verified');
persistFeatureUpdate(featureId, { status: 'verified' });
toast.success('Feature verified', {
description: `Marked as verified: ${draggedFeature.description.slice(
Expand All @@ -304,7 +299,6 @@ export function useBoardDragDrop({
// skipTests feature being moved between verified and waiting_approval
if (targetStatus === 'waiting_approval' && draggedFeature.status === 'verified') {
// Move verified feature back to waiting_approval
moveFeature(featureId, 'waiting_approval');
persistFeatureUpdate(featureId, { status: 'waiting_approval' });
toast.info('Feature moved back', {
description: `Moved back to Waiting Approval: ${draggedFeature.description.slice(
Expand All @@ -314,7 +308,6 @@ export function useBoardDragDrop({
});
} else if (targetStatus === 'backlog') {
// Allow moving skipTests cards back to backlog (from verified)
moveFeature(featureId, 'backlog');
persistFeatureUpdate(featureId, { status: 'backlog' });
toast.info('Feature moved to backlog', {
description: `Moved to Backlog: ${draggedFeature.description.slice(
Expand All @@ -327,7 +320,6 @@ export function useBoardDragDrop({
// Handle verified TDD (non-skipTests) features being moved back
if (targetStatus === 'waiting_approval') {
// Move verified feature back to waiting_approval
moveFeature(featureId, 'waiting_approval');
persistFeatureUpdate(featureId, { status: 'waiting_approval' });
toast.info('Feature moved back', {
description: `Moved back to Waiting Approval: ${draggedFeature.description.slice(
Expand All @@ -337,7 +329,6 @@ export function useBoardDragDrop({
});
} else if (targetStatus === 'backlog') {
// Allow moving verified cards back to backlog
moveFeature(featureId, 'backlog');
persistFeatureUpdate(featureId, { status: 'backlog' });
toast.info('Feature moved to backlog', {
description: `Moved to Backlog: ${draggedFeature.description.slice(
Expand All @@ -351,7 +342,6 @@ export function useBoardDragDrop({
[
features,
runningAutoTasks,
moveFeature,
updateFeature,
persistFeatureUpdate,
handleStartImplementation,
Expand Down
Loading
Loading