mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-03-18 10:23:07 +00:00
Fixes critical React crash on the Kanban board view (#830)
* Changes from fix/board-react-crash * fix: Prevent cascading re-renders and crashes from high-frequency WS events
This commit is contained in:
gsxdsm
@@ -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,
|
||||
@@ -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');
|
||||
|
||||
@@ -116,8 +118,6 @@ 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
|
||||
@@ -125,7 +125,6 @@ export function useBoardActions({
|
||||
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);
|
||||
@@ -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,
|
||||
@@ -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(
|
||||
@@ -742,7 +740,6 @@ export function useBoardActions({
|
||||
|
||||
const handleManualVerify = useCallback(
|
||||
(feature: Feature) => {
|
||||
moveFeature(feature.id, 'verified');
|
||||
persistFeatureUpdate(feature.id, {
|
||||
status: 'verified',
|
||||
justFinishedAt: undefined,
|
||||
@@ -751,7 +748,7 @@ export function useBoardActions({
|
||||
description: `Marked as verified: ${truncateDescription(feature.description)}`,
|
||||
});
|
||||
},
|
||||
[moveFeature, persistFeatureUpdate]
|
||||
[persistFeatureUpdate]
|
||||
);
|
||||
|
||||
const handleMoveBackToInProgress = useCallback(
|
||||
@@ -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(
|
||||
@@ -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)}`,
|
||||
@@ -907,7 +902,7 @@ export function useBoardActions({
|
||||
await loadFeatures();
|
||||
}
|
||||
},
|
||||
[currentProject, moveFeature, persistFeatureUpdate, loadFeatures, onWorktreeCreated]
|
||||
[currentProject, persistFeatureUpdate, loadFeatures, onWorktreeCreated]
|
||||
);
|
||||
|
||||
const handleMergeFeature = useCallback(
|
||||
@@ -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(
|
||||
@@ -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', {
|
||||
@@ -994,13 +980,7 @@ export function useBoardActions({
|
||||
});
|
||||
}
|
||||
},
|
||||
[
|
||||
updateFeature,
|
||||
persistFeatureUpdate,
|
||||
currentWorktreeBranch,
|
||||
projectPath,
|
||||
isPrimaryWorktreeBranch,
|
||||
]
|
||||
[persistFeatureUpdate, currentWorktreeBranch, projectPath, isPrimaryWorktreeBranch]
|
||||
);
|
||||
|
||||
const handleViewOutput = useCallback(
|
||||
@@ -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);
|
||||
try {
|
||||
await stopFeature(feature.id);
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
},
|
||||
[stopFeature, moveFeature, persistFeatureUpdate, currentProject, queryClient]
|
||||
[stopFeature, persistFeatureUpdate, currentProject]
|
||||
);
|
||||
|
||||
const handleStartNextFeatures = useCallback(async () => {
|
||||
|
||||
@@ -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,
|
||||
@@ -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:
|
||||
@@ -193,12 +190,16 @@ 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');
|
||||
});
|
||||
@@ -206,9 +207,7 @@ export function useBoardColumnFeatures({
|
||||
if (hasUpdatedStatus) {
|
||||
clearRecentlyCompletedFeatures();
|
||||
}
|
||||
|
||||
prevFeatureIdsRef.current = currentIds;
|
||||
}, [features, recentlyCompletedFeatures, clearRecentlyCompletedFeatures]);
|
||||
}, [features, clearRecentlyCompletedFeatures]);
|
||||
|
||||
// Memoize column features to prevent unnecessary re-renders
|
||||
const columnFeaturesMap = useMemo(() => {
|
||||
|
||||
@@ -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
|
||||
@@ -207,7 +206,8 @@ 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') {
|
||||
@@ -215,7 +215,6 @@ export function useBoardDragDrop({
|
||||
// Server will derive workDir from feature.branchName
|
||||
await handleStartImplementation(draggedFeature);
|
||||
} else {
|
||||
moveFeature(featureId, targetStatus);
|
||||
persistFeatureUpdate(featureId, { status: targetStatus });
|
||||
}
|
||||
} else if (draggedFeature.status === 'waiting_approval') {
|
||||
@@ -223,7 +222,6 @@ export function useBoardDragDrop({
|
||||
// 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',
|
||||
@@ -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',
|
||||
@@ -269,7 +266,6 @@ export function useBoardDragDrop({
|
||||
});
|
||||
}
|
||||
}
|
||||
moveFeature(featureId, 'backlog');
|
||||
persistFeatureUpdate(featureId, { status: 'backlog' });
|
||||
toast.info(
|
||||
isRunningTask
|
||||
@@ -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(
|
||||
@@ -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(
|
||||
@@ -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(
|
||||
@@ -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(
|
||||
@@ -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(
|
||||
@@ -351,7 +342,6 @@ export function useBoardDragDrop({
|
||||
[
|
||||
features,
|
||||
runningAutoTasks,
|
||||
moveFeature,
|
||||
updateFeature,
|
||||
persistFeatureUpdate,
|
||||
handleStartImplementation,
|
||||
|
||||
@@ -87,37 +87,22 @@ export function useBoardFeatures({ currentProject }: UseBoardFeaturesProps) {
|
||||
);
|
||||
|
||||
// Subscribe to auto mode events for notifications (ding sound, toasts)
|
||||
// Note: Query invalidation is handled by useAutoModeQueryInvalidation in the root
|
||||
// Note: Query invalidation is handled by useAutoModeQueryInvalidation in the root.
|
||||
// Note: removeRunningTask is handled by useAutoMode — do NOT duplicate it here,
|
||||
// as duplicate Zustand mutations cause re-render cascades (React error #185).
|
||||
useEffect(() => {
|
||||
const api = getElectronAPI();
|
||||
if (!api?.autoMode || !currentProject) return;
|
||||
|
||||
const { removeRunningTask } = useAppStore.getState();
|
||||
const projectId = currentProject.id;
|
||||
const projectPath = currentProject.path;
|
||||
|
||||
const unsubscribe = api.autoMode.onEvent((event) => {
|
||||
// Check if event is for the current project by matching projectPath
|
||||
const eventProjectPath = ('projectPath' in event && event.projectPath) as string | undefined;
|
||||
if (eventProjectPath && eventProjectPath !== projectPath) {
|
||||
// Event is for a different project, ignore it
|
||||
logger.debug(
|
||||
`Ignoring auto mode event for different project: ${eventProjectPath} (current: ${projectPath})`
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Use event's projectPath or projectId if available, otherwise use current project
|
||||
// Board view only reacts to events for the currently selected project
|
||||
const eventProjectId = ('projectId' in event && event.projectId) || projectId;
|
||||
|
||||
// NOTE: auto_mode_feature_start and auto_mode_feature_complete are NOT handled here
|
||||
// for feature list reloading. That is handled by useAutoModeQueryInvalidation which
|
||||
// invalidates the features.all query on those events. Duplicate invalidation here
|
||||
// caused a re-render cascade through DndContext that triggered React error #185
|
||||
// (maximum update depth exceeded), crashing the board view with an infinite spinner
|
||||
// when a new feature was added and moved to in_progress.
|
||||
|
||||
if (event.type === 'auto_mode_feature_complete') {
|
||||
// Play ding sound when feature is done (unless muted)
|
||||
const { muteDoneSound } = useAppStore.getState();
|
||||
@@ -126,14 +111,7 @@ export function useBoardFeatures({ currentProject }: UseBoardFeaturesProps) {
|
||||
audio.play().catch((err) => logger.warn('Could not play ding sound:', err));
|
||||
}
|
||||
} else if (event.type === 'auto_mode_error') {
|
||||
// Remove from running tasks
|
||||
if (event.featureId) {
|
||||
const eventBranchName =
|
||||
'branchName' in event && event.branchName !== undefined ? event.branchName : null;
|
||||
removeRunningTask(eventProjectId, eventBranchName, event.featureId);
|
||||
}
|
||||
|
||||
// Show error toast
|
||||
// Show error toast (removeRunningTask is handled by useAutoMode, not here)
|
||||
const isAuthError =
|
||||
event.errorType === 'authentication' ||
|
||||
(event.error &&
|
||||
|
||||
@@ -281,6 +281,10 @@ function VirtualizedList<Item extends VirtualListItem>({
|
||||
);
|
||||
}
|
||||
|
||||
// Stable empty Set to use as default prop value. Using `new Set()` inline in
|
||||
// the destructuring creates a new reference on every render, defeating memo.
|
||||
const EMPTY_FEATURE_IDS = new Set<string>();
|
||||
|
||||
export const KanbanBoard = memo(function KanbanBoard({
|
||||
activeFeature,
|
||||
getColumnFeatures,
|
||||
@@ -317,7 +321,7 @@ export const KanbanBoard = memo(function KanbanBoard({
|
||||
onOpenPipelineSettings,
|
||||
isSelectionMode = false,
|
||||
selectionTarget = null,
|
||||
selectedFeatureIds = new Set(),
|
||||
selectedFeatureIds = EMPTY_FEATURE_IDS,
|
||||
onToggleFeatureSelection,
|
||||
onToggleSelectionMode,
|
||||
onAiSuggest,
|
||||
|
||||
Reference in New Issue
Block a user