Skip to content

Commit 1f523cf

Browse files
peppescgpeppescg
authored
fix: remove optimizer workload if optimized group is deleted (#1178)
* fix: delete optimizer workload after group * fix: unregister clients on optimizer grp after grp change * leftover * fix: avoid gcTime for possible issue during query invalidation * refactor: description group if it is optimized during deletion * fix: potential invalidate cache issue based on gcTime * fix: invalidate optimizer workload on optimizer page * test: align delete group description * fix: delete optimizer workload within group deletion * refactor: review * fix: test * refactor: handle optimizer mcp error on form
1 parent a16355e commit 1f523cf

File tree

11 files changed

+140
-31
lines changed

11 files changed

+140
-31
lines changed

‎renderer/src/common/hooks/use-cleanup-meta-optimizer.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ export function useCleanupMetaOptimizer() {
8888
}),
8989
refetchOnMount: true,
9090
staleTime: 5_000,
91+
retry: false,
9192
enabled: isExperimentalFeaturesEnabled && isMetaOptimizerEnabled,
9293
})
9394

‎renderer/src/common/hooks/use-create-optimizer-workload.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export function useCreateOptimizerWorkload() {
3333
}),
3434
refetchOnMount: true,
3535
staleTime: 5_000,
36+
retry: false,
3637
enabled: isExperimentalFeaturesEnabled && isMetaOptimizerEnabled,
3738
})
3839

‎renderer/src/common/hooks/use-optimize-group-name.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ export function useOptimizedGroupName() {
1313
...getApiV1BetaWorkloadsByNameOptions({
1414
path: { name: META_MCP_SERVER_NAME },
1515
}),
16-
staleTime: 0,
17-
gcTime: 0,
16+
retry: false,
1817
enabled: isExperimentalFeaturesEnabled && isMetaOptimizerEnabled,
1918
})
2019
return optimizerWorkloadDetail?.env_vars?.ALLOWED_GROUPS

‎renderer/src/features/clients/components/__tests__/manage-clients-button.test.tsx‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,23 @@ describe('ManageClientsButton – BDD flows', () => {
7575

7676
const user = userEvent.setup()
7777
renderWithProviders({ groupName: 'default' })
78-
await user.click(
79-
await screen.findByRole('button', { name: /manage clients/i })
80-
)
81-
await user.click(await screen.findByRole('switch', { name: 'vscode' }))
82-
await user.click(await screen.findByRole('switch', { name: /cursor/i }))
83-
await user.click(await screen.findByRole('button', { name: /save/i }))
78+
await waitFor(() => {
79+
expect(
80+
screen.getByRole('button', { name: /manage clients/i })
81+
).toBeVisible()
82+
})
83+
await user.click(screen.getByRole('button', { name: /manage clients/i }))
84+
85+
await waitFor(() => {
86+
expect(
87+
screen.getByRole('heading', {
88+
name: /manage clients/i,
89+
})
90+
).toBeVisible()
91+
})
92+
await user.click(screen.getByRole('switch', { name: 'vscode' }))
93+
await user.click(screen.getByRole('switch', { name: /cursor/i }))
94+
await user.click(screen.getByRole('button', { name: /save/i }))
8495

8596
await waitFor(() =>
8697
expect(

‎renderer/src/features/mcp-servers/components/group-actions-dropdown.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import { MoreVertical, Trash2 } from 'lucide-react'
99
import { useConfirm } from '@/common/hooks/use-confirm'
1010
import { useMutationDeleteGroup } from '../hooks/use-mutation-delete-group'
1111
import { toast } from 'sonner'
12+
import { useIsOptimizedGroupName } from '@/features/clients/hooks/use-is-optimized-group-name'
1213

1314
export function GroupActionsDropdown({ groupName }: { groupName: string }) {
15+
const isOptimizedGroupName = useIsOptimizedGroupName(groupName)
1416
const confirm = useConfirm()
1517
const { mutateAsync: deleteGroup } = useMutationDeleteGroup()
1618
return (
@@ -38,7 +40,7 @@ export function GroupActionsDropdown({ groupName }: { groupName: string }) {
3840
return
3941
}
4042
const confirmed = await confirm(
41-
'Deleting this group will permanently erase all its servers. Are you sure you want to proceed? This action cannot be undone.',
43+
`Deleting this ${isOptimizedGroupName ? 'optimized' : ''} group will permanently erase all its servers. Are you sure you want to proceed? This action cannot be undone.`,
4244
{
4345
title: 'Delete group',
4446
buttons: { yes: 'Delete', no: 'Cancel' },

‎renderer/src/features/mcp-servers/hooks/use-mutation-delete-group.ts‎

Lines changed: 100 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,116 @@
1-
import { useQueryClient } from '@tanstack/react-query'
1+
import { useMutation, useQuery } from '@tanstack/react-query'
22
import { useToastMutation } from '@/common/hooks/use-toast-mutation'
3-
import { deleteApiV1BetaGroupsByNameMutation } from '@api/@tanstack/react-query.gen'
3+
import {
4+
deleteApiV1BetaGroupsByNameMutation,
5+
deleteApiV1BetaWorkloadsByNameMutation,
6+
getApiV1BetaDiscoveryClientsQueryKey,
7+
getApiV1BetaGroupsOptions,
8+
getApiV1BetaGroupsQueryKey,
9+
getApiV1BetaWorkloadsByNameOptions,
10+
getApiV1BetaWorkloadsQueryKey,
11+
postApiV1BetaClientsUnregisterMutation,
12+
} from '@api/@tanstack/react-query.gen'
413
import { useNavigate } from '@tanstack/react-router'
514
import { trackEvent } from '@/common/lib/analytics'
6-
import { useGroups } from './use-groups'
15+
import { useFeatureFlag } from '@/common/hooks/use-feature-flag'
16+
import { featureFlagKeys } from '../../../../../utils/feature-flags'
17+
import {
18+
MCP_OPTIMIZER_GROUP_NAME,
19+
META_MCP_SERVER_NAME,
20+
} from '@/common/lib/constants'
21+
import log from 'electron-log/renderer'
22+
import { toast } from 'sonner'
23+
import { queryClient } from '@/common/lib/query-client'
724

825
export function useMutationDeleteGroup() {
9-
const queryClient = useQueryClient()
26+
const isMetaOptimizerEnabled = useFeatureFlag(featureFlagKeys.META_OPTIMIZER)
27+
const { data: optimizerWorkloadDetail } = useQuery({
28+
...getApiV1BetaWorkloadsByNameOptions({
29+
path: { name: META_MCP_SERVER_NAME },
30+
}),
31+
retry: false,
32+
enabled: isMetaOptimizerEnabled,
33+
})
34+
const optimizedGroupName = optimizerWorkloadDetail?.env_vars?.ALLOWED_GROUPS
1035
const navigate = useNavigate()
11-
const { data: groupsData } = useGroups()
36+
const { data: groupsData } = useQuery({
37+
...getApiV1BetaGroupsOptions(),
38+
})
39+
const groupsNotOptimized = groupsData?.groups?.filter(
40+
(group) => group.name !== MCP_OPTIMIZER_GROUP_NAME
41+
)
42+
const optimizerGroupClients =
43+
groupsData?.groups?.find((group) => group.name === MCP_OPTIMIZER_GROUP_NAME)
44+
?.registered_clients ?? []
45+
46+
const { mutateAsync: unregisterClients } = useMutation({
47+
...postApiV1BetaClientsUnregisterMutation(),
48+
onError: (error) => {
49+
log.error('Error unregistering clients from optimizer group', error)
50+
},
51+
onSuccess: () => {
52+
log.info('Successfully unregistered clients from optimizer group')
53+
},
54+
onSettled: () => {
55+
queryClient.invalidateQueries({
56+
queryKey: getApiV1BetaDiscoveryClientsQueryKey(),
57+
})
58+
queryClient.invalidateQueries({
59+
queryKey: getApiV1BetaGroupsQueryKey(),
60+
})
61+
},
62+
})
63+
64+
const deleteWorkload = useMutation({
65+
...deleteApiV1BetaWorkloadsByNameMutation(),
66+
onSuccess: async () => {
67+
log.info('Optimizer workload deleted on group')
68+
if (optimizerGroupClients.length > 0) {
69+
await unregisterClients({
70+
body: {
71+
names: optimizerGroupClients,
72+
groups: [MCP_OPTIMIZER_GROUP_NAME],
73+
},
74+
})
75+
}
76+
},
77+
onError: (error) => {
78+
toast.error('Failed to delete optimizer workload')
79+
log.error('Failed to delete optimizer workload', error)
80+
},
81+
onSettled: () => {
82+
queryClient.invalidateQueries({
83+
queryKey: getApiV1BetaWorkloadsQueryKey(),
84+
})
85+
86+
queryClient.invalidateQueries({
87+
queryKey: getApiV1BetaGroupsQueryKey(),
88+
})
89+
90+
queryClient.invalidateQueries({
91+
queryKey: getApiV1BetaDiscoveryClientsQueryKey(),
92+
})
93+
},
94+
})
1295

1396
return useToastMutation({
1497
...deleteApiV1BetaGroupsByNameMutation(),
15-
onSuccess: () => {
16-
const remainingGroupsCount = (groupsData?.groups?.length ?? 1) - 1
98+
onSuccess: async (_, variables) => {
99+
const deletedGroupName = variables?.path?.name
100+
const remainingGroupsCount = (groupsNotOptimized?.length ?? 1) - 1
17101
trackEvent('Group deleted', {
18102
is_default_group: 'false',
19103
remaining_groups_count: remainingGroupsCount,
20104
})
21-
queryClient.invalidateQueries({ queryKey: ['api', 'v1beta', 'groups'] })
105+
if (optimizedGroupName === deletedGroupName) {
106+
await deleteWorkload.mutateAsync({
107+
path: { name: META_MCP_SERVER_NAME },
108+
})
109+
}
110+
111+
queryClient.invalidateQueries({
112+
queryKey: getApiV1BetaGroupsQueryKey(),
113+
})
22114
navigate({ to: '/group/$groupName', params: { groupName: 'default' } })
23115
},
24116
successMsg: (variables) => `Group "${variables.path.name}" deleted`,

‎renderer/src/features/meta-mcp/components/group-selector-form.tsx‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { toast } from 'sonner'
2323
import { useMcpOptimizerClients } from '../hooks/use-mcp-optimizer-clients'
2424
import { LoadingStateDialog } from './loading-state-dialog'
2525
import { useCreateOptimizerWorkload } from '@/common/hooks/use-create-optimizer-workload'
26+
2627
interface GroupSelectorFormProps {
2728
groups: GroupWithServers[]
2829
}
@@ -36,9 +37,12 @@ type FormSchema = z.infer<typeof formSchema>
3637
export function GroupSelectorForm({
3738
groups,
3839
}: GroupSelectorFormProps): ReactElement {
39-
const { data: metaMcpConfig } = useMetaMcpConfig()
40+
const { data: metaMcpConfig, isError: isMetaMcpConfigError } =
41+
useMetaMcpConfig()
4042
const [isPending, startTransition] = useTransition()
41-
const defaultSelectedGroup = getMetaMcpOptimizedGroup(metaMcpConfig)
43+
const defaultSelectedGroup = getMetaMcpOptimizedGroup(
44+
isMetaMcpConfigError ? undefined : metaMcpConfig
45+
)
4246
const { saveGroupClients } = useMcpOptimizerClients()
4347
const { handleCreateMetaOptimizerWorkload } = useCreateOptimizerWorkload()
4448
const { updateServerMutation } = useUpdateServer(META_MCP_SERVER_NAME, {
@@ -59,7 +63,7 @@ export function GroupSelectorForm({
5963
const onSubmit = async (data: FormSchema) => {
6064
startTransition(async () => {
6165
try {
62-
if (metaMcpConfig) {
66+
if (metaMcpConfig && !isMetaMcpConfigError) {
6367
const previousGroupName =
6468
metaMcpConfig?.env_vars?.[ALLOWED_GROUPS_ENV_VAR]
6569
const envVars = [
@@ -97,8 +101,6 @@ export function GroupSelectorForm({
97101
`MCP Optimizer for ${data.selectedGroup} is available`
98102
)
99103
}
100-
101-
// form.reset({ selectedGroup: data.selectedGroup })
102104
},
103105
onSettled: () => {
104106
toast.dismiss(toastId)

‎renderer/src/features/meta-mcp/hooks/use-mcp-optimizer-clients.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,10 @@ export function useMcpOptimizerClients() {
135135
const clientsToAdd = selectedGroupClients.filter(
136136
(client) => !currentOptimizerClients.includes(client)
137137
)
138-
const clientsToRemove = currentOptimizerClients.filter(
139-
(client) => clientsStatus?.[getClientFieldName(client)] === false
138+
const clientsToRemove = currentOptimizerClients.filter((client) =>
139+
clientsStatus
140+
? clientsStatus[getClientFieldName(client)] === false
141+
: !selectedGroupClients.includes(client) && isGroupChanged
140142
)
141143
log.info(
142144
`Clients to add to optimizer group: ${clientsToAdd.join(', ') || 'none'}`

‎renderer/src/features/meta-mcp/hooks/use-meta-mcp-config.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ export function useMetaMcpConfig() {
88
path: { name: META_MCP_SERVER_NAME },
99
}),
1010
retry: false,
11-
staleTime: 0,
12-
gcTime: 0,
1311
})
1412
}
1513

‎renderer/src/routes/__tests__/group.$groupName.menu.confirm.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('Group route delete group confirmation', () => {
5555
})
5656
expect(
5757
screen.getByText(
58-
'Deleting this group will permanently erase all its servers. Are you sure you want to proceed? This action cannot be undone.'
58+
/Deleting this (optimized )?group will permanently erase all its servers/i
5959
)
6060
).toBeVisible()
6161

0 commit comments

Comments
 (0)