Skip to content

Commit c278450

Browse files
waleedlatif1waleedlatif1
committed
fix(desktop): keep background chat signals behind the flag and to workspace chats
- Turn-complete notifications for chats in the background follow the background executor flag, so a flag-off user sees exactly what they saw before. - Only a workspace chat's turn binds to a desktop. Its sidebar shows the status and an approval notification links back to it; an organization chat stays with its chat view. - A desktop's status reads as running when Sim cannot track presence at all, not as blocked. - "Running on <device>" shows in the chat row's tooltip. The status dot sits in the row's indicator slot, which takes no pointer and gives way to the row's actions on hover, so a tooltip on the dot itself could never open.
1 parent 69c4dfb commit c278450

7 files changed

Lines changed: 134 additions & 36 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -201,23 +201,19 @@ interface DesktopActivityDotProps {
201201
activity: DesktopChatActivity
202202
}
203203

204-
/** The status of a chat one of the user's desktops is running in the background. */
204+
/**
205+
* The status of a chat one of the user's desktops is running in the background. The row's
206+
* indicator slot is pointer-inert and gives way to the row's actions on hover, so the label is
207+
* read out here and shown in the row's own tooltip.
208+
*/
205209
function DesktopActivityDot({ activity }: DesktopActivityDotProps) {
206-
const label = desktopActivityLabel(activity)
207210
return (
208-
<Tooltip.Root>
209-
<Tooltip.Trigger asChild>
210-
<span
211-
role='img'
212-
aria-label={label}
213-
className='size-[6px] rounded-full'
214-
style={{ backgroundColor: DESKTOP_ACTIVITY_COLOR[activity.state] }}
215-
/>
216-
</Tooltip.Trigger>
217-
<Tooltip.Content>
218-
<p>{label}</p>
219-
</Tooltip.Content>
220-
</Tooltip.Root>
211+
<span
212+
role='img'
213+
aria-label={desktopActivityLabel(activity)}
214+
className='size-[6px] rounded-full'
215+
style={{ backgroundColor: DESKTOP_ACTIVITY_COLOR[activity.state] }}
216+
/>
221217
)
222218
}
223219

@@ -280,7 +276,12 @@ const SidebarChatItem = memo(function SidebarChatItem({
280276
}
281277

282278
return (
283-
<SidebarTooltip label={chat.name} enabled={showCollapsedTooltips}>
279+
<SidebarTooltip
280+
label={
281+
desktopActivity ? `${chat.name} · ${desktopActivityLabel(desktopActivity)}` : chat.name
282+
}
283+
enabled={showCollapsedTooltips || Boolean(desktopActivity)}
284+
>
284285
<ChatNavigationLink
285286
chatId={chat.id}
286287
href={chat.href}
@@ -885,8 +886,12 @@ export const Sidebar = memo(function Sidebar({ organizationHref }: SidebarProps)
885886
{ enabled: chatEnabled && !permissionConfig.hideCopilot }
886887
)
887888

888-
useMothershipChatEvents(workspaceId, chatEnabled && !permissionConfig.hideCopilot)
889889
const desktopExecutorEnabled = useFeatureFlag('mothership-desktop-background-executor')
890+
useMothershipChatEvents(
891+
workspaceId,
892+
chatEnabled && !permissionConfig.hideCopilot,
893+
desktopExecutorEnabled
894+
)
890895
const { data: desktopActivity } = useDesktopActivity(
891896
workspaceId,
892897
desktopExecutorEnabled && chatEnabled && !permissionConfig.hideCopilot

‎apps/sim/hooks/use-mothership-chat-events.test.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ describe('reflectBackgroundChatStatus', () => {
317317
it('announces a chat that finished in the background, and opens it from the notification', () => {
318318
showing('/workspace/ws-1/chat/chat-c')
319319

320-
reflectBackgroundChatStatus(queryClient, 'ws-1', completed)
320+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, true)
321321

322322
expect(notify).toHaveBeenCalledWith({
323323
title: 'Fix CI',
@@ -326,22 +326,31 @@ describe('reflectBackgroundChatStatus', () => {
326326
})
327327
})
328328

329+
it('announces nothing while the background executor is off', () => {
330+
showing('/workspace/ws-1/chat/chat-c')
331+
332+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, false)
333+
334+
expect(notify).not.toHaveBeenCalled()
335+
})
336+
329337
it('leaves the chat on screen to announce itself', () => {
330338
showing('/workspace/ws-1/chat/chat-b')
331339

332-
reflectBackgroundChatStatus(queryClient, 'ws-1', completed)
340+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, true)
333341

334342
expect(notify).not.toHaveBeenCalled()
335343
})
336344

337345
it('stays silent outside the desktop app and for a turn that only started', () => {
338346
showing('/workspace/ws-1/chat/chat-c', false)
339-
reflectBackgroundChatStatus(queryClient, 'ws-1', completed)
347+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, true)
340348
showing('/workspace/ws-1/chat/chat-c')
341349
reflectBackgroundChatStatus(
342350
queryClient,
343351
'ws-1',
344-
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-2' })
352+
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-2' }),
353+
true
345354
)
346355

347356
expect(notify).not.toHaveBeenCalled()
@@ -353,12 +362,14 @@ describe('reflectBackgroundChatStatus', () => {
353362
reflectBackgroundChatStatus(
354363
queryClient,
355364
'ws-1',
356-
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-3' })
365+
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-3' }),
366+
true
357367
)
358368
reflectBackgroundChatStatus(
359369
queryClient,
360370
'ws-1',
361-
JSON.stringify({ chatId: 'chat-b', type: 'renamed' })
371+
JSON.stringify({ chatId: 'chat-b', type: 'renamed' }),
372+
true
362373
)
363374

364375
expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(1)

‎apps/sim/hooks/use-mothership-chat-events.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect } from 'react'
1+
import { useEffect, useRef } from 'react'
22
import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import type { QueryClient } from '@tanstack/react-query'
@@ -194,19 +194,21 @@ function chatRoute(owner: MothershipChatOwner, chatId: string): string {
194194

195195
/**
196196
* Reflects a turn starting or ending in the chats that run in the background: the desktop
197-
* activity list changes, and a chat the user is not looking at that finished its turn is
198-
* announced. The desktop app decides whether to show that notification (notifications on, the
199-
* chat not on screen in the focused window); the chat on screen announces its own completion.
197+
* activity list changes and, with `announceCompletions`, a chat the user is not looking at that
198+
* finished its turn is announced. The desktop app decides whether to show that notification
199+
* (notifications on, the chat not on screen in the focused window); the chat on screen announces
200+
* its own completion.
200201
*/
201202
export function reflectBackgroundChatStatus(
202203
queryClient: Pick<QueryClient, 'getQueryData' | 'invalidateQueries'>,
203204
owner: MothershipChatOwner,
204-
data: unknown
205+
data: unknown,
206+
announceCompletions: boolean
205207
): void {
206208
const payload = parseChatStatusEventPayload(data)
207209
if (payload?.type !== 'started' && payload?.type !== 'completed') return
208210
queryClient.invalidateQueries({ queryKey: desktopActivityKeys.lists() })
209-
if (payload.type !== 'completed' || !payload.chatId) return
211+
if (!announceCompletions || payload.type !== 'completed' || !payload.chatId) return
210212
const settings = getDesktopBridge()?.settings
211213
if (!settings) return
212214
const route = chatRoute(owner, payload.chatId)
@@ -234,9 +236,13 @@ export function reflectBackgroundChatStatus(
234236
*/
235237
export function useMothershipChatEvents(
236238
owner: MothershipChatOwner | undefined,
237-
chatEnabled: boolean
239+
chatEnabled: boolean,
240+
/** Announce chats that finish in the background; only with the background executor on. */
241+
announceBackgroundCompletions = false
238242
) {
239243
const queryClient = useQueryClient()
244+
const announceRef = useRef(announceBackgroundCompletions)
245+
announceRef.current = announceBackgroundCompletions
240246
const workspaceId = typeof owner === 'string' ? owner : undefined
241247
const organizationId = typeof owner === 'object' ? owner.organizationId : undefined
242248

@@ -255,7 +261,7 @@ export function useMothershipChatEvents(
255261
task_status: (event) => {
256262
const data = event instanceof MessageEvent ? event.data : undefined
257263
handleMothershipChatStatusEvent(queryClient, eventOwner, data)
258-
reflectBackgroundChatStatus(queryClient, eventOwner, data)
264+
reflectBackgroundChatStatus(queryClient, eventOwner, data, announceRef.current)
259265
},
260266
},
261267
onOpen: (reason) => {

‎apps/sim/lib/desktop/application/activity.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
const { presence, rows } = vi.hoisted(() => ({
5+
presence: { available: vi.fn(), present: vi.fn() },
6+
rows: vi.fn(),
7+
}))
8+
9+
vi.mock('@/lib/desktop/executor/presence', () => ({
10+
isDesktopPresenceAvailable: presence.available,
11+
isDesktopPresent: presence.present,
12+
}))
13+
vi.mock('@/lib/desktop/executor/repository', () => ({ listDesktopActivityRows: rows }))
14+
15+
import { listDesktopActivity } from '@/lib/desktop/application/activity'
16+
17+
const principal = createSessionPrincipal({ userId: 'user-1', sessionId: 'session-1' })
18+
19+
describe('desktop activity presence', () => {
20+
beforeEach(() => {
21+
rows.mockResolvedValue([
22+
{ chatId: 'chat-1', deviceId: 'device-1', deviceName: 'Studio Mac', needsInput: false },
23+
])
24+
})
25+
26+
it('does not call a desktop blocked when Sim cannot track presence at all', async () => {
27+
presence.available.mockReturnValue(false)
28+
presence.present.mockResolvedValue(false)
29+
30+
const { chats } = await listDesktopActivity.execute({
31+
principal,
32+
input: { workspaceId: 'ws-1' },
33+
})
34+
35+
expect(chats).toEqual([{ chatId: 'chat-1', state: 'running', deviceName: 'Studio Mac' }])
36+
})
37+
38+
it('calls a desktop blocked when presence is tracked and it is gone', async () => {
39+
presence.available.mockReturnValue(true)
40+
presence.present.mockResolvedValue(false)
41+
42+
const { chats } = await listDesktopActivity.execute({
43+
principal,
44+
input: { workspaceId: 'ws-1' },
45+
})
46+
47+
expect(chats).toEqual([{ chatId: 'chat-1', state: 'blocked', deviceName: 'Studio Mac' }])
48+
})
49+
})

‎apps/sim/lib/desktop/application/activity.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { defineOperation } from '@/lib/core/application'
55
import { defineAuthorizedCredentialUserUseCase } from '@/lib/credentials/application/authorized-user-use-case'
6-
import { isDesktopPresent } from '@/lib/desktop/executor/presence'
6+
import { isDesktopPresenceAvailable, isDesktopPresent } from '@/lib/desktop/executor/presence'
77
import { listDesktopActivityRows } from '@/lib/desktop/executor/repository'
88

99
const logger = createLogger('DesktopActivity')
@@ -21,8 +21,9 @@ export interface DesktopChatActivityEntry {
2121
deviceName: string
2222
}
2323

24-
/** Presence Sim cannot read is not evidence of an offline desktop. */
24+
/** Presence Sim cannot read, or cannot track at all, is not evidence of an offline desktop. */
2525
async function readPresence(deviceId: string): Promise<boolean> {
26+
if (!isDesktopPresenceAvailable()) return true
2627
try {
2728
return await isDesktopPresent(deviceId)
2829
} catch (error) {

‎apps/sim/lib/mothership/chat/application/admit-turn.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { admitChatTurn } from '@/lib/mothership/chat/application/admit-turn'
2626

2727
const hoisted = vi.hoisted(() => ({
2828
lease: vi.fn(),
29+
resolveDesktop: vi.fn(),
2930
}))
3031
const mocks = {
3132
...hoisted,
@@ -43,6 +44,9 @@ vi.mock('@/lib/mothership/request/session/controller-lease', () => ({
4344
vi.mock('@/lib/auth/ban', () => authBanMock)
4445
vi.mock('@/lib/permission-groups/resolve.server', () => permissionGroupsResolveMock)
4546
vi.mock('@/lib/mothership/chat-status', () => mothershipChatStatusMock)
47+
vi.mock('@/lib/desktop/application/executor', () => ({
48+
resolveTurnDesktopDevice: hoisted.resolveDesktop,
49+
}))
4650

4751
const principal = createSessionPrincipal({ userId: 'actor', sessionId: 'session' })
4852
const chatId = '11111111-1111-4111-8111-111111111111'
@@ -119,6 +123,25 @@ describe('organization turn admission through current private-chat authorization
119123
expect.anything()
120124
)
121125
})
126+
it("keeps an organization chat's turn with its chat view, never on a desktop", async () => {
127+
hoisted.resolveDesktop.mockResolvedValue('device-1')
128+
queueTableRows(copilotChats, [chat])
129+
queueTableRows(member, [{ role: 'member' }])
130+
dbChainMockFns.returning
131+
.mockResolvedValueOnce([{ model: null }])
132+
.mockResolvedValueOnce([{ id: 'run-1', organizationId: 'org-1', workspaceId: null }])
133+
.mockResolvedValueOnce([{ key: 'claim' }])
134+
135+
await admitChatTurn.execute({
136+
principal,
137+
input: { ...input(), desktopDeviceId: '33333333-3333-4333-8333-333333333333' },
138+
})
139+
140+
expect(hoisted.resolveDesktop).not.toHaveBeenCalled()
141+
expect(dbChainMockFns.values).toHaveBeenCalledWith(
142+
expect.objectContaining({ organizationId: 'org-1', desktopDeviceId: null })
143+
)
144+
})
122145
it.each(['agent', 'assistant', 'plan'] as const)(
123146
'switches the same chat to %s atomically with turn admission',
124147
async (mode) => {

‎apps/sim/lib/mothership/chat/application/admit-turn.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,12 @@ export const admitChatTurn = defineAuthorizedChatUseCase({
8686
else await requireOrganizationSearchAvailable(organizationId)
8787
}
8888
await assertChatStreamLease(input.lease)
89-
const desktopDeviceId = input.desktopDeviceId
90-
? await resolveTurnDesktopDevice(principal, input.desktopDeviceId)
91-
: null
89+
// Only a workspace chat runs on a desktop in the background: its sidebar shows the status and
90+
// an approval notification links back to it. An organization chat stays with its chat view.
91+
const desktopDeviceId =
92+
input.desktopDeviceId && workspaceId
93+
? await resolveTurnDesktopDevice(principal, input.desktopDeviceId)
94+
: null
9295
const turnConfig = sql`COALESCE(${copilotChats.config}, '{}'::jsonb) || jsonb_build_object('conversationMode', ${request.mode ?? 'agent'}::text)`
9396
return withRunAdmissionLock(userId, request.messageId, async (tx) => {
9497
const [chat] = await tx

0 commit comments

Comments
 (0)