From 0a7e52326c29b3e93639e56e2e9967b8dde024b6 Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Fri, 15 May 2026 15:35:27 +0000 Subject: [PATCH] batch3 T6 review fixes: remove rollback closure, flush-error resync - remove: capture snapshot inside setPanes functional updater to avoid stale-closure rollback under concurrent renders - flushPendingState: call refresh() on PATCH failure so server truth and optimistic local state can't silently diverge - Drop body.state! non-null assertion via narrowed local Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/web/src/hooks/usePanes.ts | 50 +++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/apps/web/src/hooks/usePanes.ts b/apps/web/src/hooks/usePanes.ts index e8f2238..0f4481b 100644 --- a/apps/web/src/hooks/usePanes.ts +++ b/apps/web/src/hooks/usePanes.ts @@ -19,21 +19,6 @@ export function usePanes(sessionId: string | undefined): { const pendingState = useRef>(new Map()); const debounceTimer = useRef | null>(null); - const flushPendingState = useCallback(() => { - if (debounceTimer.current !== null) { - clearTimeout(debounceTimer.current); - debounceTimer.current = null; - } - const snapshot = new Map(pendingState.current); - pendingState.current.clear(); - for (const [id, state] of snapshot) { - // fire-and-forget; caller surface handles errors via hook error state - void api.panes.update(id, { state }).catch((err) => { - setError(err instanceof Error ? err.message : 'pane operation failed'); - }); - } - }, []); - const refresh = useCallback(async () => { if (!sessionId) { setPanes(null); @@ -51,6 +36,23 @@ export function usePanes(sessionId: string | undefined): { } }, [sessionId]); + const flushPendingState = useCallback(async () => { + if (debounceTimer.current !== null) { + clearTimeout(debounceTimer.current); + debounceTimer.current = null; + } + const updates = Array.from(pendingState.current.entries()); + pendingState.current.clear(); + if (updates.length === 0) return; + try { + await Promise.all(updates.map(([id, state]) => api.panes.update(id, { state }))); + } catch (err) { + setError(err instanceof Error ? err.message : 'pane state PATCH failed'); + // server truth may diverge from optimistic local state; resync + void refresh(); + } + }, [refresh]); + // Fetch on mount / sessionId change; preserve previous list while reloading // (loading=true but panes stays non-null after first fetch to avoid flash) useEffect(() => { @@ -99,7 +101,8 @@ export function usePanes(sessionId: string | undefined): { }); // Coalesce: last state wins within debounce window - pendingState.current.set(id, body.state!); + const nextState = body.state; + pendingState.current.set(id, nextState); if (debounceTimer.current !== null) { clearTimeout(debounceTimer.current); @@ -124,21 +127,24 @@ export function usePanes(sessionId: string | undefined): { const remove = useCallback( async (id: string): Promise => { - // Optimistic remove - const previous = panes; - setPanes((prev) => (prev ? prev.filter((p) => p.id !== id) : prev)); + // Optimistic remove — capture snapshot inside functional updater to avoid stale closure + let snapshot: Pane[] | null = null; + setPanes((prev) => { + snapshot = prev; + return prev ? prev.filter((p) => p.id !== id) : prev; + }); try { await api.panes.remove(id); await refresh(); } catch (err) { - // Rollback - setPanes(previous); + // Rollback to the truly-most-recent value captured above + setPanes(snapshot); setError(err instanceof Error ? err.message : 'pane operation failed'); throw err; } }, - [panes, refresh] + [refresh] ); return { panes, loading, error, refresh, create, update, remove };