From b0a32ae4736d5643f2f59fa369219b23db514c4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 09:12:06 +0000 Subject: [PATCH] compose layout writes against current state, not a captured tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every mutation in WorkspaceView computed its new tree from the `layout` its callback closed over, and two of the paths are not immediate: the resize debounce fires 500 ms after the drag began, and a window resize fires onLayout on every group at once. So the later write was computed from a tree that predated the earlier one and silently undid it — remove a panel just after dragging a splitter and it came back. Worse now that panel identity lives in the layout: the resurrected tree carries an older `config`, so a panel that was just given an agent's name reverts to anonymous and the agent stops being addressable through it. All eight now pass an updater to setValue, which composes against the current cache. The debounce timer also had no cleanup at all, so it outlived the component. It now flushes on unmount rather than dropping — with an updater the early write is correct, and dropping would lose a splitter drag made just before navigating away, which the no-cleanup version did at least persist. Neither file is prettier-clean at HEAD, so neither was formatted; the new code is written to match. Co-Authored-By: Claude Opus 5 --- .../Workspace/WorkspaceRenderer.tsx | 28 +++++++++++-- .../components/Workspace/WorkspaceView.tsx | 42 ++++++++++++------- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx b/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx index c98a19de..038d89e0 100644 --- a/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx +++ b/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx @@ -1,4 +1,4 @@ -import { useCallback, useRef } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; import { ResizablePanel, ResizablePanelGroup, ResizableHandle } from '@/components/ui/resizable'; import type { LayoutNode, AppRegistryMap, PanelComponents } from './types'; import { countPanels } from './layout-utils'; @@ -105,9 +105,26 @@ const LayoutNodeRenderer = ({ onRemove, onResized, }: LayoutNodeRendererProps) => { - const debounceRef = useRef>(null); + const debounceRef = useRef | null>(null); + const pendingRef = useRef<(() => void) | null>(null); const mountedRef = useRef(false); + // The timer had no cleanup at all, so it outlived the component and fired against whatever it had + // captured. Flush rather than drop: `onResized` composes against current state now (WorkspaceView passes + // an updater), so running it early is correct — and dropping it would lose a splitter drag made just + // before navigating away, which the old no-cleanup version did at least persist. + useEffect( + () => () => { + if (!debounceRef.current) return; + clearTimeout(debounceRef.current); + debounceRef.current = null; + const pending = pendingRef.current; + pendingRef.current = null; + pending?.(); + }, + [], + ); + const handleLayout = useCallback( (sizes: number[]) => { if (node.type !== 'group') return; @@ -116,8 +133,13 @@ const LayoutNodeRenderer = ({ return; } if (debounceRef.current) clearTimeout(debounceRef.current); + const groupId = node.id; + pendingRef.current = () => onResized(groupId, sizes); debounceRef.current = setTimeout(() => { - onResized(node.id, sizes); + debounceRef.current = null; + const pending = pendingRef.current; + pendingRef.current = null; + pending?.(); }, 500); }, [node, onResized], diff --git a/src/workspaces/officerdev/src/components/Workspace/WorkspaceView.tsx b/src/workspaces/officerdev/src/components/Workspace/WorkspaceView.tsx index e38a85b4..d2638f3b 100644 --- a/src/workspaces/officerdev/src/components/Workspace/WorkspaceView.tsx +++ b/src/workspaces/officerdev/src/components/Workspace/WorkspaceView.tsx @@ -57,39 +57,51 @@ export const WorkspaceView = ({ workspace, locked, cwd = '~', root, initialFileP } }, [maximizedPanelId]); + // Every mutation goes through `onLayoutChange` as an UPDATER, never as a computed tree. + // + // Each of these used to close over `layout` as it was when the callback was made, and two of the paths + // are not immediate: the resize debounce fires 500 ms after the drag began, and a window resize fires + // `onLayout` on every group at once. So the second write was computed from a tree that predated the + // first and silently undid it — remove a panel just after dragging a splitter and it came back. That is + // worse than it sounds now that panel identity lives in the layout: the resurrected tree carries an + // older `config`, so a panel that was just given an agent's name reverts to anonymous, and the agent it + // addresses is no longer reachable through it. + // + // The guards below still test the rendered `layout` — they only decide whether a write is worth making, + // and being one render stale there costs a redundant no-op write at worst. const handleSetApp = useCallback( (panelId: string, appType: string | null) => { - onLayoutChange(setApp(layout, panelId, appType)); + onLayoutChange((prev) => setApp(prev, panelId, appType)); }, - [layout, onLayoutChange], + [onLayoutChange], ); const handleSplit = useCallback( (panelId: string, direction: 'horizontal' | 'vertical') => { - onLayoutChange(splitPanel(layout, panelId, direction)); + onLayoutChange((prev) => splitPanel(prev, panelId, direction)); }, - [layout, onLayoutChange], + [onLayoutChange], ); const handleRemove = useCallback( (panelId: string) => { if (countPanels(layout) <= 1) return; - onLayoutChange(removePanel(layout, panelId)); + onLayoutChange((prev) => (countPanels(prev) <= 1 ? prev : removePanel(prev, panelId))); }, [layout, onLayoutChange], ); const handleResized = useCallback( (groupId: string, sizes: number[]) => { - onLayoutChange(updateSizes(layout, groupId, sizes)); + onLayoutChange((prev) => updateSizes(prev, groupId, sizes)); }, - [layout, onLayoutChange], + [onLayoutChange], ); const handleSetZoom = useCallback( (panelId: string, zoom: number) => { - const next = setZoom(layout, panelId, zoom); - if (next !== layout) onLayoutChange(next); + if (setZoom(layout, panelId, zoom) === layout) return; + onLayoutChange((prev) => setZoom(prev, panelId, zoom)); }, [layout, onLayoutChange], ); @@ -99,26 +111,26 @@ export const WorkspaceView = ({ workspace, locked, cwd = '~', root, initialFileP const handleSetPanelConfig = useCallback( (panelId: string, config: PanelConfig | undefined) => { - const next = setPanelConfig(layout, panelId, config); - if (next !== layout) onLayoutChange(next); + if (setPanelConfig(layout, panelId, config) === layout) return; + onLayoutChange((prev) => setPanelConfig(prev, panelId, config)); }, [layout, onLayoutChange], ); const handleSwap = useCallback( (sourceId: string, targetId: string) => { - onLayoutChange(swapPanels(layout, sourceId, targetId)); + onLayoutChange((prev) => swapPanels(prev, sourceId, targetId)); setSwapSourceId(null); }, - [layout, onLayoutChange], + [onLayoutChange], ); const handleMove = useCallback( (sourceId: string, targetId: string, position: DropPosition) => { - onLayoutChange(movePanel(layout, sourceId, targetId, position)); + onLayoutChange((prev) => movePanel(prev, sourceId, targetId, position)); setDragSourceId(null); }, - [layout, onLayoutChange], + [onLayoutChange], ); const startDrag = useCallback(