Fixes critical React crash on the Kanban board view#830
Merged
gsxdsm merged 2 commits intoAutoMaker-Org:v1.0.0rcfrom Mar 4, 2026
Merged
Fixes critical React crash on the Kanban board view#830gsxdsm merged 2 commits intoAutoMaker-Org:v1.0.0rcfrom
gsxdsm merged 2 commits intoAutoMaker-Org:v1.0.0rcfrom
Conversation
Contributor
📝 WalkthroughWalkthroughRefactors board-view optimistic updates to use persistent updates and a module-level feature transition guard; stabilizes auto-mode/worktree selectors and batching; adds idempotent store guards and minor component memoization. Introduces logic to avoid WebSocket-triggered refetches during in-flight feature transitions. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant Board as Board Hook
participant Store as App Store
participant WS as WebSocket
participant Server as API/Server
User->>Board: Trigger feature action (e.g., stop/verify/move)
Board->>Board: markFeatureTransitioning(featureId)
Board->>Server: persistFeatureUpdate(featureId, updatedFields)
Server-->>Board: ACK / updated feature
Board->>Store: update local state via persistFeatureUpdate result
Board->>Board: unmarkFeatureTransitioning(featureId)
WS-->>Board: emits feature-updated event
alt feature was transitioning
Board->>Board: isAnyFeatureTransitioning() returns false? (guard prevents refetch)
else not transitioning
Board->>Store: invalidate/refetch features list
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the performance and stability of the development server and UI by addressing issues related to excessive logging, redundant state updates, and cascading React re-renders. The changes focus on optimizing event handling, centralizing state management, and introducing mechanisms to prevent race conditions, ultimately leading to a more responsive and resource-efficient application experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a series of significant performance optimizations and stability improvements across the application. The changes effectively reduce log spam, decrease event frequency, and eliminate sources of re-render cascades in the UI, which directly addresses the reported server and browser slowdowns. The refactoring to centralize state updates and the introduction of a state transition guard to prevent race conditions are particularly well-executed. I've included a couple of minor suggestions for further improvement, but overall this is a very high-quality contribution.
Comment on lines
198
to
+210
| 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]); |
Contributor
There was a problem hiding this comment.
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]); |
| // 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); |
Contributor
There was a problem hiding this comment.
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.
gsxdsm
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/views/board-view/hooks/use-board-actions.ts (1)
741-751:⚠️ Potential issue | 🟡 MinorGuard project-null paths before showing success toasts.
These handlers can show success even when
persistFeatureUpdateno-ops (whencurrentProjectis unset). Please gate on project presence (and preferably await persistence) before success feedback.Suggested pattern (apply to all affected handlers)
- const handleManualVerify = useCallback( - (feature: Feature) => { - persistFeatureUpdate(feature.id, { - status: 'verified', - justFinishedAt: undefined, - }); - toast.success('Feature verified', { - description: `Marked as verified: ${truncateDescription(feature.description)}`, - }); - }, - [persistFeatureUpdate] - ); + const handleManualVerify = useCallback( + async (feature: Feature) => { + if (!currentProject) return; + await persistFeatureUpdate(feature.id, { + status: 'verified', + justFinishedAt: undefined, + }); + toast.success('Feature verified', { + description: `Marked as verified: ${truncateDescription(feature.description)}`, + }); + }, + [currentProject, persistFeatureUpdate] + );Also applies to: 754-766, 947-955, 957-984
🤖 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 741 - 751, The success toast in handleManualVerify is shown even when persistFeatureUpdate is a no-op because currentProject may be unset; update handleManualVerify (and the other affected handlers) to first check that currentProject is present and only proceed if so, then await persistFeatureUpdate(feature.id, {...}) before calling toast.success, and handle errors (eg. show an error toast) if persistence fails; reference the persistFeatureUpdate function and currentProject variable and apply the same pattern to the other handlers listed (lines ~754-766, ~947-955, ~957-984).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ui/src/components/views/board-view/hooks/use-board-actions.ts`:
- Around line 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.
In `@apps/ui/src/hooks/use-query-invalidation.ts`:
- Around line 182-187: The guard uses isAnyFeatureTransitioning() to skip
invalidation for all features, which suppresses unrelated updates; change the
logic to scope the transition check to the specific feature referenced by the
event: when FEATURE_LIST_INVALIDATION_EVENTS.includes(event.type) only skip
invalidation if the feature for this event is transitioning (e.g., call
isFeatureTransitioning(event.featureId) or modify isAnyFeatureTransitioning to
accept an id), then invalidate the cache key "features.all" for other
events/features so unrelated feature updates are not dropped.
---
Outside diff comments:
In `@apps/ui/src/components/views/board-view/hooks/use-board-actions.ts`:
- Around line 741-751: The success toast in handleManualVerify is shown even
when persistFeatureUpdate is a no-op because currentProject may be unset; update
handleManualVerify (and the other affected handlers) to first check that
currentProject is present and only proceed if so, then await
persistFeatureUpdate(feature.id, {...}) before calling toast.success, and handle
errors (eg. show an error toast) if persistence fails; reference the
persistFeatureUpdate function and currentProject variable and apply the same
pattern to the other handlers listed (lines ~754-766, ~947-955, ~957-984).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/ui/src/components/views/board-view/hooks/use-board-actions.tsapps/ui/src/components/views/board-view/hooks/use-board-column-features.tsapps/ui/src/components/views/board-view/hooks/use-board-drag-drop.tsapps/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/hooks/use-auto-mode.tsapps/ui/src/hooks/use-query-invalidation.tsapps/ui/src/lib/feature-transition-state.tsapps/ui/src/store/app-store.ts
Comment on lines
+1014
to
+1020
| // 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); |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+182
to
+187
| // Invalidate feature list for lifecycle events. | ||
| // Skip invalidation when a feature is mid-transition (e.g., being cancelled) | ||
| // because persistFeatureUpdate already handles the optimistic cache update. | ||
| // Without this guard, auto_mode_error / auto_mode_stopped WS events race | ||
| // with the optimistic update and cause re-render cascades on mobile (React #185). | ||
| if (FEATURE_LIST_INVALIDATION_EVENTS.includes(event.type) && !isAnyFeatureTransitioning()) { |
Contributor
There was a problem hiding this comment.
Global transition guard can suppress unrelated feature invalidations.
At Line 187, using isAnyFeatureTransitioning() blocks features.all invalidation for all events while any feature is transitioning. That can drop updates for other features and leave stale board state.
🛠️ Suggested fix (scope the guard to the event’s feature only)
--- a/apps/ui/src/hooks/use-query-invalidation.ts
+++ b/apps/ui/src/hooks/use-query-invalidation.ts
@@
-import { isAnyFeatureTransitioning } from '@/lib/feature-transition-state';
+import { isFeatureTransitioning } from '@/lib/feature-transition-state';
@@
- if (FEATURE_LIST_INVALIDATION_EVENTS.includes(event.type) && !isAnyFeatureTransitioning()) {
+ const shouldSkipFeatureListInvalidation =
+ hasFeatureId(event) && isFeatureTransitioning(event.featureId);
+ if (
+ FEATURE_LIST_INVALIDATION_EVENTS.includes(event.type) &&
+ !shouldSkipFeatureListInvalidation
+ ) {
queryClient.invalidateQueries({
queryKey: queryKeys.features.all(currentProjectPath),
});
}--- a/apps/ui/src/lib/feature-transition-state.ts
+++ b/apps/ui/src/lib/feature-transition-state.ts
@@
export function isAnyFeatureTransitioning(): boolean {
return transitioningFeatures.size > 0;
}
+
+export function isFeatureTransitioning(featureId: string): boolean {
+ return transitioningFeatures.has(featureId);
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/src/hooks/use-query-invalidation.ts` around lines 182 - 187, The
guard uses isAnyFeatureTransitioning() to skip invalidation for all features,
which suppresses unrelated updates; change the logic to scope the transition
check to the specific feature referenced by the event: when
FEATURE_LIST_INVALIDATION_EVENTS.includes(event.type) only skip invalidation if
the feature for this event is transitioning (e.g., call
isFeatureTransitioning(event.featureId) or modify isAnyFeatureTransitioning to
accept an id), then invalidate the cache key "features.all" for other
events/features so unrelated feature updates are not dropped.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/hooks/use-auto-mode.ts (1)
517-525:⚠️ Potential issue | 🟠 MajorTransition guard is scoped by branch only; include project scope too.
Line 517 and Line 558 gate transition behavior using only
eventBranchName === branchName. Since this listener is global, events from another project on the same branch (especiallynullmain worktree) can incorrectly clear/suppress this hook’s transition state.Proposed fix: gate by both project and branch
@@ + const isCurrentHookWorktreeEvent = + eventProjectId === projectId && eventBranchName === branchName; @@ - if (isRestartTransitionRef.current && eventBranchName === branchName) { + if (isRestartTransitionRef.current && isCurrentHookWorktreeEvent) { @@ - if (eventBranchName === branchName && isTransitioningRef.current) { + if (isCurrentHookWorktreeEvent && isTransitioningRef.current) {Also applies to: 558-566
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/hooks/use-auto-mode.ts` around lines 517 - 525, The transition guard currently only checks eventBranchName === branchName (in the block around isRestartTransitionRef.current and the other block at lines ~558-566); update both guards to also validate the project scope by checking eventProjectName === projectName (i.e., require eventProjectName === projectName && eventBranchName === branchName) so the listener ignores events from other projects (including null/main worktree collisions); modify the conditions in the blocks that reference isRestartTransitionRef.current and isTransitioningRef.current accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ui/src/hooks/use-auto-mode.ts`:
- Around line 221-239: The current batchedAddAutoModeActivity collects
activities but still calls addAutoModeActivity for each item, so it doesn't
batch Zustand set() calls; change batchedAddAutoModeActivity to, when the flush
timer fires, call a new store action addAutoModeActivities(batch) once (passing
the entire batch) instead of looping and invoking addAutoModeActivity for each
item, and implement addAutoModeActivities(activities: AutoModeActivity[]) inside
the app-store (apps/ui/src/store/app-store.ts) to append all activities in a
single set(...) call; keep pendingActivitiesRef and flushTimerRef logic the same
but replace the per-item flush loop with a single call to addAutoModeActivities.
- Around line 246-253: The cleanup currently clears flushTimerRef but drops any
buffered entries in pendingActivitiesRef; update the useEffect cleanup to drain
the pending queue before unmount by invoking the same flush routine used
elsewhere (e.g., call flushPendingActivities or the function that
processes/sends pendingActivitiesRef.current) or, if no helper exists, iterate
pendingActivitiesRef.current, process/send each entry, then clear the array and
clearTimeout(flushTimerRef.current); reference useEffect, flushTimerRef, and
pendingActivitiesRef to locate and modify the cleanup block.
---
Outside diff comments:
In `@apps/ui/src/hooks/use-auto-mode.ts`:
- Around line 517-525: The transition guard currently only checks
eventBranchName === branchName (in the block around
isRestartTransitionRef.current and the other block at lines ~558-566); update
both guards to also validate the project scope by checking eventProjectName ===
projectName (i.e., require eventProjectName === projectName && eventBranchName
=== branchName) so the listener ignores events from other projects (including
null/main worktree collisions); modify the conditions in the blocks that
reference isRestartTransitionRef.current and isTransitioningRef.current
accordingly.
Comment on lines
+221
to
+239
| // Batch addAutoModeActivity calls to reduce Zustand set() frequency. | ||
| // Without batching, each WS event (especially auto_mode_progress which fires | ||
| // rapidly during streaming) triggers a separate set() → all subscriber selectors | ||
| // re-evaluate → on mobile this overwhelms React's batching → crash. | ||
| // This batches activities in a ref and flushes them in a single set() call. | ||
| const pendingActivitiesRef = useRef<Parameters<typeof addAutoModeActivity>[0][]>([]); | ||
| const flushTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
| const batchedAddAutoModeActivity = useCallback( | ||
| (activity: Parameters<typeof addAutoModeActivity>[0]) => { | ||
| pendingActivitiesRef.current.push(activity); | ||
| if (!flushTimerRef.current) { | ||
| flushTimerRef.current = setTimeout(() => { | ||
| const batch = pendingActivitiesRef.current; | ||
| pendingActivitiesRef.current = []; | ||
| flushTimerRef.current = null; | ||
| // Flush all pending activities in a single store update | ||
| for (const act of batch) { | ||
| addAutoModeActivity(act); | ||
| } |
Contributor
There was a problem hiding this comment.
batchedAddAutoModeActivity is not actually batching store writes.
Line 237-239 still calls addAutoModeActivity once per item, and each call performs its own Zustand set(). This keeps update count proportional to event count and undermines the stated crash mitigation.
Proposed fix: flush with a single store action call
@@
- addAutoModeActivity,
+ addAutoModeActivities,
@@
- addAutoModeActivity: state.addAutoModeActivity,
+ addAutoModeActivities: state.addAutoModeActivities,
@@
- const pendingActivitiesRef = useRef<Parameters<typeof addAutoModeActivity>[0][]>([]);
+ const pendingActivitiesRef = useRef<Parameters<typeof addAutoModeActivities>[0]>([]);
@@
- (activity: Parameters<typeof addAutoModeActivity>[0]) => {
+ (activity: Parameters<typeof addAutoModeActivities>[0][number]) => {
@@
- // Flush all pending activities in a single store update
- for (const act of batch) {
- addAutoModeActivity(act);
- }
+ // Flush all pending activities in a single store update
+ addAutoModeActivities(batch);
@@
- [addAutoModeActivity]
+ [addAutoModeActivities]You’ll also need a matching addAutoModeActivities(activities[]) action in apps/ui/src/store/app-store.ts that appends the whole batch inside one set(...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/src/hooks/use-auto-mode.ts` around lines 221 - 239, The current
batchedAddAutoModeActivity collects activities but still calls
addAutoModeActivity for each item, so it doesn't batch Zustand set() calls;
change batchedAddAutoModeActivity to, when the flush timer fires, call a new
store action addAutoModeActivities(batch) once (passing the entire batch)
instead of looping and invoking addAutoModeActivity for each item, and implement
addAutoModeActivities(activities: AutoModeActivity[]) inside the app-store
(apps/ui/src/store/app-store.ts) to append all activities in a single set(...)
call; keep pendingActivitiesRef and flushTimerRef logic the same but replace the
per-item flush loop with a single call to addAutoModeActivities.
Comment on lines
+246
to
+253
| // Cleanup flush timer on unmount | ||
| useEffect(() => { | ||
| return () => { | ||
| if (flushTimerRef.current) { | ||
| clearTimeout(flushTimerRef.current); | ||
| } | ||
| }; | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Queued activities are dropped on unmount.
Line 249-251 clears the flush timer but never drains pendingActivitiesRef. Any buffered activity entries are silently lost during quick unmounts/worktree switches.
Proposed fix: flush pending queue during cleanup
- useEffect(() => {
+ useEffect(() => {
return () => {
if (flushTimerRef.current) {
clearTimeout(flushTimerRef.current);
+ flushTimerRef.current = null;
}
+ if (pendingActivitiesRef.current.length > 0) {
+ const batch = pendingActivitiesRef.current;
+ pendingActivitiesRef.current = [];
+ // Prefer single-call batch action if available
+ for (const act of batch) addAutoModeActivity(act);
+ }
};
- }, []);
+ }, [addAutoModeActivity]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Cleanup flush timer on unmount | |
| useEffect(() => { | |
| return () => { | |
| if (flushTimerRef.current) { | |
| clearTimeout(flushTimerRef.current); | |
| } | |
| }; | |
| }, []); | |
| // Cleanup flush timer on unmount | |
| useEffect(() => { | |
| return () => { | |
| if (flushTimerRef.current) { | |
| clearTimeout(flushTimerRef.current); | |
| flushTimerRef.current = null; | |
| } | |
| if (pendingActivitiesRef.current.length > 0) { | |
| const batch = pendingActivitiesRef.current; | |
| pendingActivitiesRef.current = []; | |
| // Prefer single-call batch action if available | |
| for (const act of batch) addAutoModeActivity(act); | |
| } | |
| }; | |
| }, [addAutoModeActivity]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/src/hooks/use-auto-mode.ts` around lines 246 - 253, The cleanup
currently clears flushTimerRef but drops any buffered entries in
pendingActivitiesRef; update the useEffect cleanup to drain the pending queue
before unmount by invoking the same flush routine used elsewhere (e.g., call
flushPendingActivities or the function that processes/sends
pendingActivitiesRef.current) or, if no helper exists, iterate
pendingActivitiesRef.current, process/send each entry, then clear the array and
clearTimeout(flushTimerRef.current); reference useEffect, flushTimerRef, and
pendingActivitiesRef to locate and modify the cleanup block.
gsxdsm
added a commit
to gsxdsm/automaker
that referenced
this pull request
Mar 4, 2026
* Changes from fix/board-react-crash * fix: Prevent cascading re-renders and crashes from high-frequency WS events
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a critical React crash (Error #185: Maximum update depth exceeded) on the Kanban board view by eliminating redundant dual state mutations. It also includes substantial improvements to board action logic, worktree handling, and feature lifecycle management.
Root Cause
The crash was caused by redundant state mutations in board action and drag-drop handlers:
moveFeature/updateFeatureandpersistFeatureUpdate, which internally handles its own optimistic React Query cache updateuseBoardActionswas subscribing to the entire Zustand store via bareuseAppStore(), causingBoardViewto re-render on every store change and cascade into infinite effect loopsChanges
Bug Fixes
useAppStore()destructuring with individual selectors (useAppStore((s) => s.field)) to prevent subscribing to the entire storemoveFeaturecalls beforepersistFeatureUpdateinhandleManualVerify,handleMoveBackToInProgress,handleCompleteFeature,handleUnarchiveFeature,handleCommitFeature, and drag-drop handlers —persistFeatureUpdatealready handles the optimistic cache updatefeature-transition-state.tsto suppress WebSocket-driven query invalidations during feature cancel/stop transitions, preventing cache flip-flops that caused re-render cascadesstopFeaturepreviously only removed running tasks from the current worktree scope; now usesremoveRunningTaskFromAllWorktreesto clear the feature from all worktree contextsFiles Changed
use-board-actions.tsuse-board-column-features.tsuse-board-drag-drop.tsuse-board-features.tskanban-board.tsxstopFeatureprop down touseBoardActionsuse-auto-mode.tsstopFeaturefor prop-drilling to avoid duplicate subscriptionsuse-query-invalidation.tsfeature-transition-stateguardfeature-transition-state.tsapp-store.tsremoveRunningTaskcross-worktree helperSummary by CodeRabbit
Bug Fixes
Performance Improvements
Stability