Skip to content

Commit 401be2d

Browse files
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 1c498de commit 401be2d

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
@@ -295,7 +295,7 @@ describe('reflectBackgroundChatStatus', () => {
295295
it('announces a chat that finished in the background, and opens it from the notification', () => {
296296
showing('/workspace/ws-1/chat/chat-c')
297297

298-
reflectBackgroundChatStatus(queryClient, 'ws-1', completed)
298+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, true)
299299

300300
expect(notify).toHaveBeenCalledWith({
301301
title: 'Fix CI',
@@ -304,22 +304,31 @@ describe('reflectBackgroundChatStatus', () => {
304304
})
305305
})
306306

307+
it('announces nothing while the background executor is off', () => {
308+
showing('/workspace/ws-1/chat/chat-c')
309+
310+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, false)
311+
312+
expect(notify).not.toHaveBeenCalled()
313+
})
314+
307315
it('leaves the chat on screen to announce itself', () => {
308316
showing('/workspace/ws-1/chat/chat-b')
309317

310-
reflectBackgroundChatStatus(queryClient, 'ws-1', completed)
318+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, true)
311319

312320
expect(notify).not.toHaveBeenCalled()
313321
})
314322

315323
it('stays silent outside the desktop app and for a turn that only started', () => {
316324
showing('/workspace/ws-1/chat/chat-c', false)
317-
reflectBackgroundChatStatus(queryClient, 'ws-1', completed)
325+
reflectBackgroundChatStatus(queryClient, 'ws-1', completed, true)
318326
showing('/workspace/ws-1/chat/chat-c')
319327
reflectBackgroundChatStatus(
320328
queryClient,
321329
'ws-1',
322-
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-2' })
330+
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-2' }),
331+
true
323332
)
324333

325334
expect(notify).not.toHaveBeenCalled()
@@ -331,12 +340,14 @@ describe('reflectBackgroundChatStatus', () => {
331340
reflectBackgroundChatStatus(
332341
queryClient,
333342
'ws-1',
334-
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-3' })
343+
JSON.stringify({ chatId: 'chat-b', type: 'started', streamId: 's-3' }),
344+
true
335345
)
336346
reflectBackgroundChatStatus(
337347
queryClient,
338348
'ws-1',
339-
JSON.stringify({ chatId: 'chat-b', type: 'renamed' })
349+
JSON.stringify({ chatId: 'chat-b', type: 'renamed' }),
350+
true
340351
)
341352

342353
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'
@@ -189,19 +189,21 @@ function chatRoute(owner: MothershipChatOwner, chatId: string): string {
189189

190190
/**
191191
* Reflects a turn starting or ending in the chats that run in the background: the desktop
192-
* activity list changes, and a chat the user is not looking at that finished its turn is
193-
* announced. The desktop app decides whether to show that notification (notifications on, the
194-
* chat not on screen in the focused window); the chat on screen announces its own completion.
192+
* activity list changes and, with `announceCompletions`, a chat the user is not looking at that
193+
* finished its turn is announced. The desktop app decides whether to show that notification
194+
* (notifications on, the chat not on screen in the focused window); the chat on screen announces
195+
* its own completion.
195196
*/
196197
export function reflectBackgroundChatStatus(
197198
queryClient: Pick<QueryClient, 'getQueryData' | 'invalidateQueries'>,
198199
owner: MothershipChatOwner,
199-
data: unknown
200+
data: unknown,
201+
announceCompletions: boolean
200202
): void {
201203
const payload = parseChatStatusEventPayload(data)
202204
if (payload?.type !== 'started' && payload?.type !== 'completed') return
203205
queryClient.invalidateQueries({ queryKey: desktopActivityKeys.lists() })
204-
if (payload.type !== 'completed' || !payload.chatId) return
206+
if (!announceCompletions || payload.type !== 'completed' || !payload.chatId) return
205207
const settings = getDesktopBridge()?.settings
206208
if (!settings) return
207209
const route = chatRoute(owner, payload.chatId)
@@ -229,9 +231,13 @@ export function reflectBackgroundChatStatus(
229231
*/
230232
export function useMothershipChatEvents(
231233
owner: MothershipChatOwner | undefined,
232-
chatEnabled: boolean
234+
chatEnabled: boolean,
235+
/** Announce chats that finish in the background; only with the background executor on. */
236+
announceBackgroundCompletions = false
233237
) {
234238
const queryClient = useQueryClient()
239+
const announceRef = useRef(announceBackgroundCompletions)
240+
announceRef.current = announceBackgroundCompletions
235241
const workspaceId = typeof owner === 'string' ? owner : undefined
236242
const organizationId = typeof owner === 'object' ? owner.organizationId : undefined
237243

@@ -250,7 +256,7 @@ export function useMothershipChatEvents(
250256
task_status: (event) => {
251257
const data = event instanceof MessageEvent ? event.data : undefined
252258
handleMothershipChatStatusEvent(queryClient, eventOwner, data)
253-
reflectBackgroundChatStatus(queryClient, eventOwner, data)
259+
reflectBackgroundChatStatus(queryClient, eventOwner, data, announceRef.current)
254260
},
255261
},
256262
onOpen: (reason) => {
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)