From 6722335a79382e4e6674d63ca7a040f1d4d13e8d Mon Sep 17 00:00:00 2001 From: Anso Date: Tue, 19 May 2026 17:56:41 -0400 Subject: [PATCH] fix(stack-update): refresh frontend state automatically after a stack update (#1113) After applying a stack update the sidebar's "update available" dot stayed visible and the stack's status indicator was stuck on the optimistic value until the page was manually refreshed. Two root causes: 1. Image-updates state refresh was a fire-and-forget call in some paths and entirely missing from the bulk-update, auto-update, and state-invalidate WebSocket-handler paths. 2. stackActionsRef.current was resynced only at render time, so the post- update refreshStacks(true) running in the action's finally block read a stale "busy" map and preserved the optimistic mask via prev[file] ?? status. Backend now broadcasts a state-invalidate event with scope='image-updates' and action='stack-updated' after every successful update (single-stack route and auto-update loop). The frontend useNotifications hook routes this to a new onImageUpdatesChange callback wired to fetchImageUpdates in EditorLayout, so every connected client refreshes the dot through the same code path. Bulk update also calls fetchImageUpdates directly for fast local feedback, and setStackAction/clearStackAction now keep stackActionsRef synchronously in sync with state so the busy-stack check inside refreshStacks observes the cleared map immediately. Adds 3 unit tests covering the new WS branch (positive, scope-mismatch negative, auto-update-settings-changed negative). --- backend/src/routes/imageUpdates.ts | 9 +++ backend/src/routes/stacks.ts | 8 +++ frontend/src/components/EditorLayout.tsx | 2 + .../hooks/useNotifications.test.ts | 72 +++++++++++++++++-- .../EditorLayout/hooks/useNotifications.ts | 11 ++- .../EditorLayout/hooks/useStackListState.ts | 27 ++++--- 6 files changed, 113 insertions(+), 16 deletions(-) diff --git a/backend/src/routes/imageUpdates.ts b/backend/src/routes/imageUpdates.ts index bd92ff8c..3e1cd56f 100644 --- a/backend/src/routes/imageUpdates.ts +++ b/backend/src/routes/imageUpdates.ts @@ -292,6 +292,15 @@ autoUpdateRouter.post('/execute', authMiddleware, async (req: Request, res: Resp await compose.updateStack(stackName, undefined, atomic); db.clearStackUpdateStatus(req.nodeId, stackName); + NotificationService.getInstance().broadcastEvent({ + type: 'state-invalidate', + scope: 'image-updates', + nodeId: req.nodeId, + stackName, + action: 'stack-updated', + ts: Date.now(), + }); + NotificationService.getInstance().dispatchAlert( 'info', 'image_update_applied', diff --git a/backend/src/routes/stacks.ts b/backend/src/routes/stacks.ts index 81e87639..1c9adb22 100644 --- a/backend/src/routes/stacks.ts +++ b/backend/src/routes/stacks.ts @@ -834,6 +834,14 @@ stacksRouter.post('/:stackName/update', async (req: Request, res: Response) => { await ComposeService.getInstance(req.nodeId).updateStack(stackName, getTerminalWs(), atomic); DatabaseService.getInstance().clearStackUpdateStatus(req.nodeId, stackName); invalidateNodeCaches(req.nodeId); + NotificationService.getInstance().broadcastEvent({ + type: 'state-invalidate', + scope: 'image-updates', + nodeId: req.nodeId, + stackName, + action: 'stack-updated', + ts: Date.now(), + }); console.log(`[Stacks] Update completed: ${sanitizeForLog(stackName)}`); if (debug) console.debug(`[Stacks:debug] Update finished in ${Date.now() - t0}ms`); res.json({ status: 'Update completed' }); diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index e94273ce..bc49040f 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -83,6 +83,7 @@ export default function EditorLayout() { scheduleStateInvalidateRefresh, toggleBulkMode, toggleSelect, clearSelection, handleBulkAction, stackUpdates, + fetchImageUpdates, pinned, isCollapsed, toggleCollapse, remoteSearchLoading, @@ -130,6 +131,7 @@ export default function EditorLayout() { nodes, onStateInvalidate: scheduleStateInvalidateRefresh, onAutoUpdateChange: fetchAutoUpdateSettings, + onImageUpdatesChange: fetchImageUpdates, }); const containerStats = useContainerStats(containers); diff --git a/frontend/src/components/EditorLayout/hooks/useNotifications.test.ts b/frontend/src/components/EditorLayout/hooks/useNotifications.test.ts index 10834c5c..b64181f4 100644 --- a/frontend/src/components/EditorLayout/hooks/useNotifications.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useNotifications.test.ts @@ -60,7 +60,7 @@ describe('useNotifications', () => { it('starts with empty notifications and disconnected state', () => { const { result } = renderHook(() => - useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn() }), + useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn(), onImageUpdatesChange: vi.fn() }), ); expect(result.current.notifications).toEqual([]); expect(result.current.tickerConnected).toBe(false); @@ -68,7 +68,7 @@ describe('useNotifications', () => { it('opens a local notification WebSocket on mount', () => { renderHook(() => - useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn() }), + useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn(), onImageUpdatesChange: vi.fn() }), ); expect(MockWS.instances.length).toBeGreaterThanOrEqual(1); expect(MockWS.instances[0]).toBeDefined(); @@ -76,7 +76,7 @@ describe('useNotifications', () => { it('sets tickerConnected true when local WS opens', () => { const { result } = renderHook(() => - useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn() }), + useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn(), onImageUpdatesChange: vi.fn() }), ); act(() => { MockWS.instances[0]?.onopen?.(); }); expect(result.current.tickerConnected).toBe(true); @@ -84,7 +84,7 @@ describe('useNotifications', () => { it('adds notification when local WS receives notification message', () => { const { result } = renderHook(() => - useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn() }), + useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn(), onImageUpdatesChange: vi.fn() }), ); act(() => { MockWS.instances[0]?.onopen?.(); }); act(() => { @@ -98,7 +98,7 @@ describe('useNotifications', () => { it('clearAllNotifications empties the local state', async () => { const { result } = renderHook(() => - useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn() }), + useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn(), onImageUpdatesChange: vi.fn() }), ); act(() => { MockWS.instances[0]?.onopen?.(); }); act(() => { @@ -111,9 +111,69 @@ describe('useNotifications', () => { await waitFor(() => expect(result.current.notifications).toHaveLength(0)); }); + it('fires onImageUpdatesChange on state-invalidate with action="stack-updated"', () => { + const onStateInvalidate = vi.fn(); + const onImageUpdatesChange = vi.fn(); + const onAutoUpdateChange = vi.fn(); + renderHook(() => + useNotifications({ nodes: [localNode], onStateInvalidate, onAutoUpdateChange, onImageUpdatesChange }), + ); + act(() => { MockWS.instances[0]?.onopen?.(); }); + act(() => { + MockWS.instances[0]?.onmessage?.({ + data: JSON.stringify({ + type: 'state-invalidate', scope: 'image-updates', nodeId: 1, + stackName: 'foo', action: 'stack-updated', ts: 1000, + }), + }); + }); + expect(onImageUpdatesChange).toHaveBeenCalledTimes(1); + expect(onStateInvalidate).toHaveBeenCalledTimes(1); + expect(onAutoUpdateChange).not.toHaveBeenCalled(); + }); + + it('does not fire onImageUpdatesChange on a generic state-invalidate', () => { + const onStateInvalidate = vi.fn(); + const onImageUpdatesChange = vi.fn(); + renderHook(() => + useNotifications({ nodes: [localNode], onStateInvalidate, onAutoUpdateChange: vi.fn(), onImageUpdatesChange }), + ); + act(() => { MockWS.instances[0]?.onopen?.(); }); + act(() => { + MockWS.instances[0]?.onmessage?.({ + data: JSON.stringify({ + type: 'state-invalidate', scope: 'stack', nodeId: 1, + stackName: 'foo', action: 'start', ts: 1000, + }), + }); + }); + expect(onStateInvalidate).toHaveBeenCalledTimes(1); + expect(onImageUpdatesChange).not.toHaveBeenCalled(); + }); + + it('does not fire onImageUpdatesChange on auto-update-settings-changed', () => { + const onAutoUpdateChange = vi.fn(); + const onImageUpdatesChange = vi.fn(); + const onStateInvalidate = vi.fn(); + renderHook(() => + useNotifications({ nodes: [localNode], onStateInvalidate, onAutoUpdateChange, onImageUpdatesChange }), + ); + act(() => { MockWS.instances[0]?.onopen?.(); }); + act(() => { + MockWS.instances[0]?.onmessage?.({ + data: JSON.stringify({ + type: 'state-invalidate', nodeId: 1, action: 'auto-update-settings-changed', ts: 1000, + }), + }); + }); + expect(onAutoUpdateChange).toHaveBeenCalledTimes(1); + expect(onImageUpdatesChange).not.toHaveBeenCalled(); + expect(onStateInvalidate).not.toHaveBeenCalled(); + }); + it('deleteNotification removes the matching item', async () => { const { result } = renderHook(() => - useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn() }), + useNotifications({ nodes: [localNode], onStateInvalidate: vi.fn(), onAutoUpdateChange: vi.fn(), onImageUpdatesChange: vi.fn() }), ); act(() => { MockWS.instances[0]?.onopen?.(); }); const notif = makeNotif({ id: 5, nodeId: localNode.id }); diff --git a/frontend/src/components/EditorLayout/hooks/useNotifications.ts b/frontend/src/components/EditorLayout/hooks/useNotifications.ts index 0f421590..9f8c0e78 100644 --- a/frontend/src/components/EditorLayout/hooks/useNotifications.ts +++ b/frontend/src/components/EditorLayout/hooks/useNotifications.ts @@ -8,9 +8,10 @@ interface UseNotificationsOptions { nodes: Node[]; onStateInvalidate: () => void; onAutoUpdateChange: () => void; + onImageUpdatesChange: () => void; } -export function useNotifications({ nodes, onStateInvalidate, onAutoUpdateChange }: UseNotificationsOptions) { +export function useNotifications({ nodes, onStateInvalidate, onAutoUpdateChange, onImageUpdatesChange }: UseNotificationsOptions) { const [notifications, setNotifications] = useState([]); const [tickerConnected, setTickerConnected] = useState(false); @@ -23,6 +24,8 @@ export function useNotifications({ nodes, onStateInvalidate, onAutoUpdateChange onStateInvalidateRef.current = onStateInvalidate; const onAutoUpdateChangeRef = useRef(onAutoUpdateChange); onAutoUpdateChangeRef.current = onAutoUpdateChange; + const onImageUpdatesChangeRef = useRef(onImageUpdatesChange); + onImageUpdatesChangeRef.current = onImageUpdatesChange; const fetchNotifications = async () => { try { @@ -98,6 +101,9 @@ export function useNotifications({ nodes, onStateInvalidate, onAutoUpdateChange onAutoUpdateChangeRef.current(); } else { onStateInvalidateRef.current(); + if (msg.scope === 'image-updates' && msg.action === 'stack-updated') { + onImageUpdatesChangeRef.current(); + } } } } catch (e) { @@ -171,6 +177,9 @@ export function useNotifications({ nodes, onStateInvalidate, onAutoUpdateChange } else if (msg.type === 'state-invalidate') { window.dispatchEvent(new CustomEvent('sencho:state-invalidate', { detail: { ...msg, nodeId: rn.id } })); onStateInvalidateRef.current(); + if (msg.scope === 'image-updates' && msg.action === 'stack-updated') { + onImageUpdatesChangeRef.current(); + } } } catch (e) { console.error(`[WS notifications:${rn.name}] parse error`, e); diff --git a/frontend/src/components/EditorLayout/hooks/useStackListState.ts b/frontend/src/components/EditorLayout/hooks/useStackListState.ts index eff4f96d..d53ac502 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackListState.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackListState.ts @@ -37,7 +37,6 @@ export function useStackListState() { const [isLoading, setIsLoading] = useState(false); const [stackActions, setStackActions] = useState>({}); const stackActionsRef = useRef>({}); - stackActionsRef.current = stackActions; const [isScanning, setIsScanning] = useState(false); const [searchQuery, setSearchQuery] = useState(''); @@ -65,15 +64,21 @@ export function useStackListState() { if (evictedOldest) toast.info('Pinned. Unpinned oldest (max 10).'); }, [evictedOldest]); + // Ref is updated synchronously alongside the state setter so any code that + // runs right after (e.g. `refreshStacks(true)` in an action's finally block) + // observes the cleared map before React commits the next render. Without + // this, the busy-stack check inside refreshStacks would still flag the + // stack as in-progress and preserve the optimistic status mask. const setStackAction = (stackFile: string, action: StackAction) => { - setStackActions(prev => ({ ...prev, [stackFile]: action })); + const next = { ...stackActionsRef.current, [stackFile]: action }; + stackActionsRef.current = next; + setStackActions(next); }; const clearStackAction = (stackFile: string) => { - setStackActions(prev => { - const next = { ...prev }; - delete next[stackFile]; - return next; - }); + const next = { ...stackActionsRef.current }; + delete next[stackFile]; + stackActionsRef.current = next; + setStackActions(next); }; const isStackBusy = useCallback((stackFile: string) => stackFile in stackActionsRef.current, []); @@ -271,9 +276,13 @@ export function useStackListState() { const handleBulkAction = useCallback((action: BulkAction) => { const filesToAction = Array.from(selectedFiles); runBulk(action, filesToAction, { - onAfter: () => { refreshStacksRef.current(true); clearSelection(); }, + onAfter: () => { + refreshStacksRef.current(true); + if (action === 'update') void fetchImageUpdates(); + clearSelection(); + }, }); - }, [selectedFiles, runBulk, clearSelection]); + }, [selectedFiles, runBulk, clearSelection, fetchImageUpdates]); const chipFilteredFilesRef = useRef(chipFilteredFiles); useEffect(() => { chipFilteredFilesRef.current = chipFilteredFiles; }, [chipFilteredFiles]);