From 64af066a116dd965a9df74bf680e3ada54af9924 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 29 Jun 2026 12:42:50 -0400 Subject: [PATCH 1/4] fix(multiuser): keep queue badge & progress bar live across users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In multiuser mode, queue item status changes and batch-enqueued events are emitted only to the owning user's socket room (for privacy). The frontend badge and progress bar only refetch the global queue status in response to those events, so a non-admin only learns about queue changes from their *own* jobs. Once their jobs finish they get no further signal: the global total (the "/Y" in "X/Y") and the in_progress count freeze, leaving the badge wrong and the progress bar animating indefinitely. Broadcast a content-free `queue_counts_changed` event to the whole queue room whenever counts change (a status transition or an enqueue). It carries only the queue_id — no user_id, batch_id, session_id, or counts — so every subscriber can refetch GET /queue/{id}/status, which already redacts per-user data. Nothing private is leaked, and it never fires on high-frequency invocation progress events. Co-Authored-By: Claude Opus 4.8 (1M context) --- invokeai/app/api/sockets.py | 23 +++++++++++++++++++ .../src/services/events/setEventListeners.tsx | 10 ++++++++ .../frontend/web/src/services/events/types.ts | 5 ++++ 3 files changed, 38 insertions(+) diff --git a/invokeai/app/api/sockets.py b/invokeai/app/api/sockets.py index 7e93c332d64..144a1e9a2c0 100644 --- a/invokeai/app/api/sockets.py +++ b/invokeai/app/api/sockets.py @@ -260,6 +260,21 @@ async def _handle_sub_bulk_download(self, sid: str, data: Any) -> None: async def _handle_unsub_bulk_download(self, sid: str, data: Any) -> None: await self._sio.leave_room(sid, BulkDownloadSubscriptionEvent(**data).bulk_download_id) + async def _broadcast_queue_counts_changed(self, queue_id: str): + """Broadcast a content-free signal that the queue's aggregate counts may have changed. + + The detailed queue item events (status changes, enqueues) are private to their owner + and admins, so non-admin subscribers never learn when *other* users' jobs are queued, + started, or finished. Without that signal their badge's global total — and the + in_progress count that drives the progress animation — go stale and get stuck once + their own jobs finish. + + This event carries nothing but the queue_id: no user_id, batch_id, session_id, or + counts. Every subscriber simply refetches GET /queue/{queue_id}/status, which already + redacts per-user data, so broadcasting it to the whole queue room leaks nothing. + """ + await self._sio.emit(event="queue_counts_changed", data={"queue_id": queue_id}, room=queue_id) + async def _handle_queue_event(self, event: FastAPIEvent[QueueEventBase]): """Handle queue events with user isolation. @@ -313,6 +328,10 @@ async def _handle_queue_event(self, event: FastAPIEvent[QueueEventBase]): logger.debug(f"Emitted private queue item event {event_name} to user room {user_room} and admin room") + # A status change shifts the global pending/in_progress totals, so nudge every + # subscriber to refetch their (redacted) queue status — see _broadcast_queue_counts_changed. + await self._broadcast_queue_counts_changed(event_data.queue_id) + # RecallParametersUpdatedEvent is private - only emit to owner + admins. # # Emit to the union of the owner room and the admin room in a SINGLE @@ -340,6 +359,10 @@ async def _handle_queue_event(self, event: FastAPIEvent[QueueEventBase]): await self._sio.emit(event=event_name, data=event_data.model_dump(mode="json"), room="admin") logger.debug(f"Emitted private batch_enqueued event to user room {user_room} and admin room") + # Newly enqueued items raise the global total, so nudge every subscriber to + # refetch their (redacted) queue status — see _broadcast_queue_counts_changed. + await self._broadcast_queue_counts_changed(event_data.queue_id) + else: # For remaining queue events (e.g. QueueClearedEvent) that do not # carry user identity, emit to all subscribers in the queue room. diff --git a/invokeai/frontend/web/src/services/events/setEventListeners.tsx b/invokeai/frontend/web/src/services/events/setEventListeners.tsx index e6010ce4ca1..05b23e74428 100644 --- a/invokeai/frontend/web/src/services/events/setEventListeners.tsx +++ b/invokeai/frontend/web/src/services/events/setEventListeners.tsx @@ -451,6 +451,16 @@ export const setEventListeners = ({ socket, store, setIsConnected }: SetEventLis } }); + socket.on('queue_counts_changed', (data) => { + // A content-free broadcast that the queue's global counts may have changed — typically + // because *another* user enqueued or one of their jobs changed status. We never receive + // those users' private queue item events, so this is the only signal that lets a non-admin's + // badge keep its global total (the "/Y" in "X/Y") and the in_progress-driven progress bar + // current. Only the redacted SessionQueueStatus is refetched; no per-user/batch caches. + log.trace({ data }, 'Queue counts changed'); + dispatch(queueApi.util.invalidateTags(['SessionQueueStatus'])); + }); + socket.on('queue_cleared', (data) => { log.debug({ data }, 'Queue cleared'); dispatch( diff --git a/invokeai/frontend/web/src/services/events/types.ts b/invokeai/frontend/web/src/services/events/types.ts index 8937dcc451d..e8dff693cb4 100644 --- a/invokeai/frontend/web/src/services/events/types.ts +++ b/invokeai/frontend/web/src/services/events/types.ts @@ -26,6 +26,11 @@ export type ServerToClientEvents = { model_install_cancelled: (payload: S['ModelInstallCancelledEvent']) => void; model_load_complete: (payload: S['ModelLoadCompleteEvent']) => void; queue_item_status_changed: (payload: S['QueueItemStatusChangedEvent']) => void; + // Content-free broadcast to the whole queue room: the global queue counts may have changed + // (some user enqueued or a job changed status). Carries only queue_id — no per-user data — + // so every subscriber can refetch the redacted queue status. Emitted by the backend socket + // layer, not the event bus, so it has no generated `S[...]` schema type. + queue_counts_changed: (payload: { queue_id: string }) => void; queue_cleared: (payload: S['QueueClearedEvent']) => void; batch_enqueued: (payload: S['BatchEnqueuedEvent']) => void; queue_items_retried: (payload: S['QueueItemsRetriedEvent']) => void; From 7e4127f52b794e7eb150404666bbb0042b25d79a Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 29 Jun 2026 13:17:15 -0400 Subject: [PATCH 2/4] fix(multiuser): don't hijack admin's gallery with other users' images In multiuser mode, admins are subscribed to the "admin" socket room and so receive invocation_complete events for every user. addImagesToGallery ran for all of them, so any image generated by another user would insert into the admin's gallery and auto-switch the admin's selected board to that user's board, displaying their image. This did not happen between two unprivileged users because they never receive each other's events. Gate addImagesToGallery on ownership: when an authenticated user is present and the event's user_id differs from theirs, skip the gallery insertion and auto-switch entirely. In single-user mode there is no authenticated user, so the original behavior is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/services/events/onInvocationComplete.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx b/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx index ea6a237d4b2..0c83fa718f0 100644 --- a/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx +++ b/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx @@ -1,5 +1,6 @@ import { logger } from 'app/logging/logger'; import type { AppDispatch, AppGetState } from 'app/store/store'; +import { selectCurrentUser } from 'features/auth/store/authSlice'; import { canvasWorkflowIntegrationProcessingCompleted } from 'features/controlLayers/store/canvasWorkflowIntegrationSlice'; import { selectAutoSwitch, @@ -45,6 +46,20 @@ export const buildOnInvocationComplete = ( completedInvocationKeysByItemId: Map> ) => { const addImagesToGallery = async (data: S['InvocationCompleteEvent']) => { + // In multiuser mode, admins are subscribed to the "admin" socket room and therefore receive + // invocation events for *every* user, not just their own. Those images belong to another + // user's gallery and boards — we must not insert them into this client's gallery or, worse, + // auto-switch the selected board to the other user's board and select their image. + // + // Only gate this when we actually know who is logged in. In single-user mode there is no + // authenticated user (selectCurrentUser is null) and every event is the local user's own, so + // we fall through and preserve the original behavior. + const currentUser = selectCurrentUser(getState()); + if (currentUser && data.user_id !== currentUser.user_id) { + log.trace(`Skipping gallery update for image owned by another user (${data.user_id})`); + return; + } + if (nodeTypeDenylist.includes(data.invocation.type)) { log.trace(`Skipping denylisted node type (${data.invocation.type})`); return; From 3ce31f3042923abc49b0f162bbe64a0bcc4cb09d Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 29 Jun 2026 21:35:05 -0400 Subject: [PATCH 3/4] test(multiuser): assert redacted queue_counts_changed is broadcast The badge-livening change emits a redacted queue_counts_changed nudge to the whole queue room on status changes and batch enqueues. Update the two routing tests to check per-event: the private event still must not reach the queue_id room, but the data-free counts nudge legitimately does. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../routers/test_multiuser_authorization.py | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/tests/app/routers/test_multiuser_authorization.py b/tests/app/routers/test_multiuser_authorization.py index 8cca29dee69..4fcf5997054 100644 --- a/tests/app/routers/test_multiuser_authorization.py +++ b/tests/app/routers/test_multiuser_authorization.py @@ -1787,15 +1787,26 @@ def test_queue_item_status_changed_routed_privately(self, socketio: Any) -> None asyncio.run(socketio._handle_queue_event(("queue_item_status_changed", event))) - rooms_emitted_to = [call.kwargs.get("room") for call in mock_emit.call_args_list] - assert "user:owner-xyz" in rooms_emitted_to - assert "admin" in rooms_emitted_to - # CRITICAL: must NOT emit to the queue_id room — that would leak to other users - assert "default" not in rooms_emitted_to + emits = [(call.kwargs.get("event"), call.kwargs.get("room")) for call in mock_emit.call_args_list] + + # The private queue_item_status_changed event carries unsanitized per-user + # metadata and must go ONLY to the owner + admin rooms. + private_rooms = [room for name, room in emits if name == "queue_item_status_changed"] + assert "user:owner-xyz" in private_rooms + assert "admin" in private_rooms + # CRITICAL: must NOT emit the private event to the queue_id room — that would leak to other users + assert "default" not in private_rooms + + # A redacted queue_counts_changed nudge (no per-user data) IS broadcast to the + # whole queue room so every subscriber's badge stays live across users. + counts_rooms = [room for name, room in emits if name == "queue_counts_changed"] + assert "default" in counts_rooms def test_batch_enqueued_routed_privately(self, socketio: Any) -> None: - """Verify that _handle_queue_event emits BatchEnqueuedEvent ONLY to - user:{user_id} and admin rooms, never to the queue_id room.""" + """Verify that _handle_queue_event emits the private BatchEnqueuedEvent ONLY to + user:{user_id} and admin rooms, never to the queue_id room. A redacted + queue_counts_changed nudge is still broadcast to the queue room so every + user's badge stays live.""" import asyncio from unittest.mock import AsyncMock @@ -1821,10 +1832,19 @@ def test_batch_enqueued_routed_privately(self, socketio: Any) -> None: asyncio.run(socketio._handle_queue_event(("batch_enqueued", event))) - rooms_emitted_to = [call.kwargs.get("room") for call in mock_emit.call_args_list] - assert "user:owner-zzz" in rooms_emitted_to - assert "admin" in rooms_emitted_to - assert "default" not in rooms_emitted_to + emits = [(call.kwargs.get("event"), call.kwargs.get("room")) for call in mock_emit.call_args_list] + + # The private batch_enqueued event carries the enqueuing user's batch_id/origin/ + # counts and must go ONLY to the owner + admin rooms. + private_rooms = [room for name, room in emits if name == "batch_enqueued"] + assert "user:owner-zzz" in private_rooms + assert "admin" in private_rooms + assert "default" not in private_rooms + + # A redacted queue_counts_changed nudge (no per-user data) IS broadcast to the + # whole queue room so every subscriber's badge stays live across users. + counts_rooms = [room for name, room in emits if name == "queue_counts_changed"] + assert "default" in counts_rooms def test_queue_cleared_still_broadcast(self, socketio: Any) -> None: """QueueClearedEvent does not carry user identity and should still be broadcast From 2f7547a2c14c6f14893b6d24c5edb51841f3c15c Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 12 Jul 2026 11:22:15 -0400 Subject: [PATCH 4/4] fix(multiuser): ignore other users' invocation_complete entirely (JPPhoto review) Admins receive every user's invocation events via the "admin" socket room. The cross-user guard only wrapped addImagesToGallery(), so another user's invocation_complete still upserted this client's $nodeExecutionStates, cleared the canvas workflow integration spinner, and reset $lastProgressEvent. Hoist the guard to the top of the returned handler so a foreign event is skipped before any side effect runs. Single-user mode (no authenticated user) still processes every event. Add a vitest covering the admin-receives-foreign, own-event, and single-user paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../events/onInvocationComplete.test.ts | 105 ++++++++++++++++++ .../services/events/onInvocationComplete.tsx | 30 ++--- 2 files changed, 121 insertions(+), 14 deletions(-) create mode 100644 invokeai/frontend/web/src/services/events/onInvocationComplete.test.ts diff --git a/invokeai/frontend/web/src/services/events/onInvocationComplete.test.ts b/invokeai/frontend/web/src/services/events/onInvocationComplete.test.ts new file mode 100644 index 00000000000..512c6bce203 --- /dev/null +++ b/invokeai/frontend/web/src/services/events/onInvocationComplete.test.ts @@ -0,0 +1,105 @@ +import type { AppDispatch, AppGetState } from 'app/store/store'; +import type { S } from 'services/api/types'; +import { $lastProgressEvent } from 'services/events/stores'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { buildOnInvocationComplete } from './onInvocationComplete'; + +vi.mock('app/logging/logger', () => ({ + logger: () => ({ + debug: vi.fn(), + trace: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})); + +// Spies for the workflow-editor node execution side effects. Kept lazy so vi.mock hoisting is safe. +const mockUpsertExecutionState = vi.fn(); +let mockNodeExecutionStates: Record = {}; +vi.mock('features/nodes/hooks/useNodeExecutionState', () => ({ + $nodeExecutionStates: { get: () => mockNodeExecutionStates }, + upsertExecutionState: (...args: unknown[]) => mockUpsertExecutionState(...args), +})); + +const mockGetUpdatedNodeExecutionState = vi.fn(); +vi.mock('services/events/nodeExecutionState', () => ({ + getUpdatedNodeExecutionStateOnInvocationComplete: (...args: unknown[]) => mockGetUpdatedNodeExecutionState(...args), +})); + +const buildEvent = (overrides: Partial = {}): S['InvocationCompleteEvent'] => + ({ + invocation: { type: 'some_node', id: 'inv-1' }, + invocation_source_id: 'node-1', + result: {}, + origin: 'workflows', + destination: 'canvas', + user_id: 'owner-1', + item_id: 1, + ...overrides, + }) as unknown as S['InvocationCompleteEvent']; + +const makeGetState = (user: { user_id: string } | null): AppGetState => + (() => ({ auth: { user } })) as unknown as AppGetState; + +const seedProgressEvent = () => { + const progress = { queue_id: 'default', item_id: 1 } as unknown as S['InvocationProgressEvent']; + $lastProgressEvent.set(progress); + return progress; +}; + +beforeEach(() => { + mockUpsertExecutionState.mockReset(); + mockGetUpdatedNodeExecutionState.mockReset(); + mockNodeExecutionStates = {}; + $lastProgressEvent.set(null); +}); + +describe('buildOnInvocationComplete cross-user isolation', () => { + it("ignores another user's completion event entirely (admin receiving via admin room)", async () => { + // An admin has a local workflow node with the same source id in progress, and a live progress event. + mockNodeExecutionStates = { 'node-1': { nodeId: 'node-1', status: 'IN_PROGRESS' } }; + mockGetUpdatedNodeExecutionState.mockReturnValue({ nodeId: 'node-1', status: 'COMPLETED' }); + const seededProgress = seedProgressEvent(); + + const dispatch = vi.fn() as unknown as AppDispatch; + const handler = buildOnInvocationComplete(makeGetState({ user_id: 'admin-1' }), dispatch, new Map()); + + // The event belongs to a different user. + await handler(buildEvent({ user_id: 'other-user', origin: 'canvas_workflow_integration' })); + + // No node execution state read/upsert, no canvas/gallery dispatches, progress event untouched. + expect(mockGetUpdatedNodeExecutionState).not.toHaveBeenCalled(); + expect(mockUpsertExecutionState).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); + expect($lastProgressEvent.get()).toBe(seededProgress); + }); + + it("processes the client's own completion event", async () => { + mockNodeExecutionStates = { 'node-1': { nodeId: 'node-1', status: 'IN_PROGRESS' } }; + mockGetUpdatedNodeExecutionState.mockReturnValue({ nodeId: 'node-1', status: 'COMPLETED' }); + seedProgressEvent(); + + const dispatch = vi.fn() as unknown as AppDispatch; + const handler = buildOnInvocationComplete(makeGetState({ user_id: 'owner-1' }), dispatch, new Map()); + + await handler(buildEvent({ user_id: 'owner-1' })); + + expect(mockUpsertExecutionState).toHaveBeenCalledWith('node-1', expect.objectContaining({ nodeId: 'node-1' })); + // The handler clears the progress event at the end when it processes an event it owns. + expect($lastProgressEvent.get()).toBeNull(); + }); + + it('processes events in single-user mode where there is no authenticated user', async () => { + mockNodeExecutionStates = { 'node-1': { nodeId: 'node-1', status: 'IN_PROGRESS' } }; + mockGetUpdatedNodeExecutionState.mockReturnValue({ nodeId: 'node-1', status: 'COMPLETED' }); + + const dispatch = vi.fn() as unknown as AppDispatch; + const handler = buildOnInvocationComplete(makeGetState(null), dispatch, new Map()); + + await handler(buildEvent({ user_id: 'system' })); + + expect(mockUpsertExecutionState).toHaveBeenCalledWith('node-1', expect.objectContaining({ nodeId: 'node-1' })); + }); +}); diff --git a/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx b/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx index 0c83fa718f0..33e97452d80 100644 --- a/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx +++ b/invokeai/frontend/web/src/services/events/onInvocationComplete.tsx @@ -46,20 +46,6 @@ export const buildOnInvocationComplete = ( completedInvocationKeysByItemId: Map> ) => { const addImagesToGallery = async (data: S['InvocationCompleteEvent']) => { - // In multiuser mode, admins are subscribed to the "admin" socket room and therefore receive - // invocation events for *every* user, not just their own. Those images belong to another - // user's gallery and boards — we must not insert them into this client's gallery or, worse, - // auto-switch the selected board to the other user's board and select their image. - // - // Only gate this when we actually know who is logged in. In single-user mode there is no - // authenticated user (selectCurrentUser is null) and every event is the local user's own, so - // we fall through and preserve the original behavior. - const currentUser = selectCurrentUser(getState()); - if (currentUser && data.user_id !== currentUser.user_id) { - log.trace(`Skipping gallery update for image owned by another user (${data.user_id})`); - return; - } - if (nodeTypeDenylist.includes(data.invocation.type)) { log.trace(`Skipping denylisted node type (${data.invocation.type})`); return; @@ -262,6 +248,22 @@ export const buildOnInvocationComplete = ( return async (data: S['InvocationCompleteEvent']) => { log.debug({ data } as JsonObject, `Invocation complete (${data.invocation.type}, ${data.invocation_source_id})`); + // In multiuser mode, admins are subscribed to the "admin" socket room and therefore receive + // invocation events for *every* user, not just their own. Another user's completion event must + // not touch this client's state: it could mark our workflow-editor nodes complete and append the + // other user's outputs to matching node IDs, stop our canvas workflow integration spinner, insert + // their images into our gallery, or clear our own in-progress progress event. Bail out entirely + // before any of those side effects run. + // + // Only gate this when we actually know who is logged in. In single-user mode there is no + // authenticated user (selectCurrentUser is null) and every event is the local user's own, so + // we fall through and preserve the original behavior. + const currentUser = selectCurrentUser(getState()); + if (currentUser && data.user_id !== currentUser.user_id) { + log.trace(`Skipping invocation complete for another user (${data.user_id})`); + return; + } + const nodeExecutionState = $nodeExecutionStates.get()[data.invocation_source_id]; const updatedNodeExecutionState = getUpdatedNodeExecutionStateOnInvocationComplete( nodeExecutionState,