fix: agent tool selector marketplace checks for local and builtin tools (#37037)

This commit is contained in:
呆萌闷油瓶 2026-06-04 14:04:09 +08:00 committed by GitHub
parent b67c3a5f76
commit b77f5f1e4a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 242 additions and 48 deletions

View File

@ -1,5 +1,6 @@
import type { ReactNode } from 'react'
import type { Node } from 'reactflow'
import type { PluginSource } from '@/app/components/plugins/types'
import type { Collection } from '@/app/components/tools/types'
import type { ToolDefaultValue, ToolValue } from '@/app/components/workflow/block-selector/types'
import type { SchemaRoot } from '@/app/components/workflow/nodes/llm/types'
@ -32,17 +33,23 @@ let mockWorkflowTools: ToolWithProvider[] | undefined = []
let mockMcpTools: ToolWithProvider[] | undefined = []
vi.mock('@/service/use-tools', () => ({
useAllBuiltInTools: () => ({ data: mockBuildInTools }),
useAllCustomTools: () => ({ data: mockCustomTools }),
useAllWorkflowTools: () => ({ data: mockWorkflowTools }),
useAllMCPTools: () => ({ data: mockMcpTools }),
useAllBuiltInTools: () => ({ data: mockBuildInTools, isFetched: true }),
useAllCustomTools: () => ({ data: mockCustomTools, isFetched: true }),
useAllWorkflowTools: () => ({ data: mockWorkflowTools, isFetched: true }),
useAllMCPTools: () => ({ data: mockMcpTools, isFetched: true }),
useInvalidateAllBuiltInTools: () => vi.fn(),
}))
// Track manifest mock state
let mockManifestData: Record<string, unknown> | null = null
let mockInstalledPlugins: Array<{ source: PluginSource }> = []
vi.mock('@/service/use-plugins', () => ({
useCheckInstalled: ({ pluginIds, enabled }: { pluginIds: string[], enabled: boolean }) => ({
data: enabled && pluginIds.length > 0
? { plugins: mockInstalledPlugins }
: undefined,
}),
usePluginManifestInfo: () => ({ data: mockManifestData }),
useInvalidateInstalledPluginList: () => vi.fn(),
}))
@ -416,11 +423,12 @@ const defaultProps = {
describe('usePluginInstalledCheck Hook', () => {
beforeEach(() => {
vi.clearAllMocks()
mockInstalledPlugins = []
})
it('should return inMarketPlace as false when manifest is null', () => {
const { result } = renderHook(
() => usePluginInstalledCheck('test-provider/tool'),
() => usePluginInstalledCheck({}),
{ wrapper: createWrapper() },
)
@ -430,7 +438,7 @@ describe('usePluginInstalledCheck Hook', () => {
it('should handle empty provider name', () => {
const { result } = renderHook(
() => usePluginInstalledCheck(''),
() => usePluginInstalledCheck({}),
{ wrapper: createWrapper() },
)
@ -439,12 +447,12 @@ describe('usePluginInstalledCheck Hook', () => {
it('should extract pluginID from provider name correctly', () => {
const { result } = renderHook(
() => usePluginInstalledCheck('org/plugin/extra'),
() => usePluginInstalledCheck({ providerPluginId: 'org/plugin' }),
{ wrapper: createWrapper() },
)
// The hook should parse "org/plugin" from "org/plugin/extra"
expect(result.current.inMarketPlace).toBe(false)
expect(result.current.pluginID).toBe('org/plugin')
})
})

View File

@ -1,7 +1,11 @@
import { renderHook } from '@testing-library/react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { PluginSource } from '@/app/components/plugins/types'
import { usePluginInstalledCheck } from '../use-plugin-installed-check'
const mockUseCheckInstalled = vi.fn()
const mockUsePluginManifestInfo = vi.fn()
const mockManifest = {
data: {
plugin: {
@ -12,52 +16,76 @@ const mockManifest = {
}
vi.mock('@/service/use-plugins', () => ({
usePluginManifestInfo: (pluginID: string) => ({
data: pluginID ? mockManifest : undefined,
}),
useCheckInstalled: (...args: unknown[]) => mockUseCheckInstalled(...args),
usePluginManifestInfo: (...args: unknown[]) => mockUsePluginManifestInfo(...args),
}))
describe('usePluginInstalledCheck', () => {
beforeEach(() => {
vi.clearAllMocks()
mockUseCheckInstalled.mockImplementation(({ pluginIds, enabled }: { pluginIds: string[], enabled: boolean }) => ({
data: enabled && pluginIds.length > 0
? { plugins: [] }
: undefined,
}))
mockUsePluginManifestInfo.mockImplementation((pluginID: string) => ({
data: pluginID ? mockManifest : undefined,
}))
})
it('should extract pluginID from provider name', () => {
const { result } = renderHook(() => usePluginInstalledCheck('org/plugin/tool'))
it('should use the explicit pluginID', () => {
const { result } = renderHook(() => usePluginInstalledCheck({
providerPluginId: 'org/plugin',
}))
expect(result.current.pluginID).toBe('org/plugin')
})
it('should detect plugin in marketplace when manifest exists', () => {
const { result } = renderHook(() => usePluginInstalledCheck('org/plugin/tool'))
const { result } = renderHook(() => usePluginInstalledCheck({
providerPluginId: 'org/plugin',
}))
expect(result.current.inMarketPlace).toBe(true)
expect(result.current.manifest).toEqual(mockManifest.data.plugin)
})
it('should handle empty provider name', () => {
const { result } = renderHook(() => usePluginInstalledCheck(''))
expect(result.current.pluginID).toBe('')
expect(result.current.inMarketPlace).toBe(false)
})
it('should handle undefined provider name', () => {
it('should handle missing plugin id', () => {
const { result } = renderHook(() => usePluginInstalledCheck())
expect(result.current.pluginID).toBe('')
expect(result.current.inMarketPlace).toBe(false)
})
it('should handle provider name with only one segment', () => {
const { result } = renderHook(() => usePluginInstalledCheck('single'))
it('should skip marketplace lookup when installed plugin source is local', () => {
mockUseCheckInstalled.mockReturnValue({
data: {
plugins: [{
source: PluginSource.local,
}],
},
})
expect(result.current.pluginID).toBe('single')
const { result } = renderHook(() => usePluginInstalledCheck({
providerPluginId: 'org/plugin',
enabled: true,
}))
expect(mockUsePluginManifestInfo).toHaveBeenCalledWith('')
expect(result.current.inMarketPlace).toBe(false)
})
it('should handle provider name with two segments', () => {
const { result } = renderHook(() => usePluginInstalledCheck('org/plugin'))
it('should skip all plugin checks for non-plugin providers', () => {
const { result } = renderHook(() => usePluginInstalledCheck({
providerPluginId: null,
enabled: true,
}))
expect(result.current.pluginID).toBe('org/plugin')
expect(mockUseCheckInstalled).toHaveBeenCalledWith({
pluginIds: [],
enabled: false,
})
expect(mockUsePluginManifestInfo).toHaveBeenCalledWith('')
expect(result.current.pluginID).toBe('')
})
})

View File

@ -1,6 +1,7 @@
import type { ToolValue } from '@/app/components/workflow/block-selector/types'
import { act, renderHook } from '@testing-library/react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { CollectionType } from '@/app/components/tools/types'
import { useToolSelectorState } from '../use-tool-selector-state'
const mockToolParams = [
@ -12,6 +13,8 @@ const mockTools = [
{
id: 'test-provider',
name: 'Test Provider',
plugin_id: 'org/test-plugin',
type: CollectionType.builtIn,
tools: [
{
name: 'test-tool',
@ -22,12 +25,13 @@ const mockTools = [
],
},
]
let areToolQueriesFetched = true
vi.mock('@/service/use-tools', () => ({
useAllBuiltInTools: () => ({ data: mockTools }),
useAllCustomTools: () => ({ data: [] }),
useAllWorkflowTools: () => ({ data: [] }),
useAllMCPTools: () => ({ data: [] }),
useAllBuiltInTools: () => ({ data: mockTools, isFetched: areToolQueriesFetched }),
useAllCustomTools: () => ({ data: [], isFetched: areToolQueriesFetched }),
useAllWorkflowTools: () => ({ data: [], isFetched: areToolQueriesFetched }),
useAllMCPTools: () => ({ data: [], isFetched: areToolQueriesFetched }),
useInvalidateAllBuiltInTools: () => vi.fn().mockResolvedValue(undefined),
}))
@ -35,12 +39,10 @@ vi.mock('@/service/use-plugins', () => ({
useInvalidateInstalledPluginList: () => vi.fn().mockResolvedValue(undefined),
}))
const mockUsePluginInstalledCheck = vi.fn()
vi.mock('../use-plugin-installed-check', () => ({
usePluginInstalledCheck: () => ({
inMarketPlace: false,
manifest: null,
pluginID: '',
}),
usePluginInstalledCheck: (...args: unknown[]) => mockUsePluginInstalledCheck(...args),
}))
vi.mock('@/utils/get-icon', () => ({
@ -64,16 +66,24 @@ describe('useToolSelectorState', () => {
const toolValue: ToolValue = {
provider_name: 'test-provider',
provider_show_name: 'Test Provider',
plugin_id: 'org/test-plugin',
tool_name: 'test-tool',
tool_label: 'Test Tool',
tool_description: 'A test tool',
settings: {},
parameters: {},
enabled: true,
type: CollectionType.builtIn,
}
beforeEach(() => {
vi.clearAllMocks()
areToolQueriesFetched = true
mockUsePluginInstalledCheck.mockReturnValue({
inMarketPlace: false,
manifest: null,
pluginID: '',
})
})
it('should initialize with default panel states', () => {
@ -221,4 +231,86 @@ describe('useToolSelectorState', () => {
expect(result.current.currentToolSettings).toEqual([])
expect(result.current.currentToolParams).toEqual([])
})
it('should skip plugin checks after resolving the current provider and tool', () => {
renderHook(() =>
useToolSelectorState({ value: toolValue, onSelect: mockOnSelect }),
)
expect(mockUsePluginInstalledCheck).toHaveBeenCalledWith({
providerPluginId: 'org/test-plugin',
enabled: false,
})
})
it('should keep plugin checks enabled when the current tool cannot be resolved', () => {
renderHook(() =>
useToolSelectorState({
value: { ...toolValue, tool_name: 'missing-tool' },
onSelect: mockOnSelect,
}),
)
expect(mockUsePluginInstalledCheck).toHaveBeenCalledWith({
providerPluginId: 'org/test-plugin',
enabled: true,
})
})
it('should keep marketplace fallback enabled after tool queries settle without resolving the provider', () => {
renderHook(() =>
useToolSelectorState({
value: {
...toolValue,
provider_name: 'org/market-plugin/search',
plugin_id: 'org/market-plugin',
},
onSelect: mockOnSelect,
}),
)
expect(mockUsePluginInstalledCheck).toHaveBeenCalledWith({
providerPluginId: 'org/market-plugin',
enabled: true,
})
})
it('should keep marketplace fallback enabled when legacy provider lists are still pending but plugin id is known', () => {
areToolQueriesFetched = false
renderHook(() =>
useToolSelectorState({
value: {
...toolValue,
provider_name: 'org/market-plugin/search',
plugin_id: 'org/market-plugin',
},
onSelect: mockOnSelect,
}),
)
expect(mockUsePluginInstalledCheck).toHaveBeenCalledWith({
providerPluginId: 'org/market-plugin',
enabled: true,
})
})
it('should skip marketplace checks for unresolved non-plugin workflow tools', () => {
renderHook(() =>
useToolSelectorState({
value: {
...toolValue,
provider_name: 'author/tool-b',
plugin_id: undefined,
type: CollectionType.workflow,
},
onSelect: mockOnSelect,
}),
)
expect(mockUsePluginInstalledCheck).toHaveBeenCalledWith({
providerPluginId: null,
enabled: true,
})
})
})

View File

@ -1,14 +1,35 @@
import { PluginSource } from '@/app/components/plugins/types'
import {
useCheckInstalled,
usePluginManifestInfo,
} from '@/service/use-plugins'
export const usePluginInstalledCheck = (providerName = '') => {
const pluginID = providerName?.split('/').splice(0, 2).join('/')
type UsePluginInstalledCheckOptions = {
providerPluginId?: string | null
enabled?: boolean
}
const { data: manifest } = usePluginManifestInfo(pluginID)
export const usePluginInstalledCheck = (
input: UsePluginInstalledCheckOptions = {},
) => {
const {
providerPluginId,
enabled = true,
} = input
const pluginID = providerPluginId ?? ''
const { data: installedPluginData } = useCheckInstalled({
pluginIds: pluginID ? [pluginID] : [],
enabled: enabled && !!pluginID,
})
const installedPlugin = installedPluginData?.plugins.at(0)
const shouldQueryMarketplace = enabled
&& !!pluginID
&& (!installedPlugin || installedPlugin.source === PluginSource.marketplace)
const { data: manifest } = usePluginManifestInfo(shouldQueryMarketplace ? pluginID : '')
return {
inMarketPlace: !!manifest,
inMarketPlace: installedPlugin?.source === PluginSource.marketplace || !!manifest,
manifest: manifest?.data.plugin,
pluginID,
}

View File

@ -4,6 +4,7 @@ import type { ToolParameter } from '@/app/components/tools/types'
import type { ToolDefaultValue, ToolValue } from '@/app/components/workflow/block-selector/types'
import type { ResourceVarInputs } from '@/app/components/workflow/nodes/_base/types'
import { useCallback, useMemo, useState } from 'react'
import { CollectionType } from '@/app/components/tools/types'
import { generateFormValue, getPlainValue, getStructureValue, toolParametersToFormSchemas } from '@/app/components/tools/utils/to-form-schema'
import { useInvalidateInstalledPluginList } from '@/service/use-plugins'
import {
@ -39,16 +40,17 @@ export const useToolSelectorState = ({
const [currType, setCurrType] = useState<TabType>('settings')
// Fetch all tools data
const { data: buildInTools } = useAllBuiltInTools()
const { data: customTools } = useAllCustomTools()
const { data: workflowTools } = useAllWorkflowTools()
const { data: mcpTools } = useAllMCPTools()
const buildInToolsQuery = useAllBuiltInTools()
const customToolsQuery = useAllCustomTools()
const workflowToolsQuery = useAllWorkflowTools()
const mcpToolsQuery = useAllMCPTools()
const { data: buildInTools } = buildInToolsQuery
const { data: customTools } = customToolsQuery
const { data: workflowTools } = workflowToolsQuery
const { data: mcpTools } = mcpToolsQuery
const invalidateAllBuiltinTools = useInvalidateAllBuiltInTools()
const invalidateInstalledPluginList = useInvalidateInstalledPluginList()
// Plugin info check
const { inMarketPlace, manifest, pluginID } = usePluginInstalledCheck(value?.provider_name)
// Merge all tools and find current provider
const currentProvider = useMemo(() => {
const mergedTools = [
@ -59,11 +61,52 @@ export const useToolSelectorState = ({
]
return mergedTools.find(toolWithProvider => toolWithProvider.id === value?.provider_name)
}, [value, buildInTools, customTools, workflowTools, mcpTools])
const areToolProvidersSettled = [
buildInToolsQuery,
customToolsQuery,
workflowToolsQuery,
mcpToolsQuery,
].every(toolProvidersQuery => toolProvidersQuery.isFetched)
// Current tool from provider
const currentTool = useMemo(() => {
return currentProvider?.tools.find(tool => tool.name === value?.tool_name)
}, [currentProvider?.tools, value?.tool_name])
const providerPluginId = useMemo(() => {
if (currentProvider)
return currentProvider.plugin_id ?? value?.plugin_id ?? null
if (value?.plugin_id)
return value.plugin_id
if (!areToolProvidersSettled || !value?.provider_name)
return undefined
// Legacy tool values may only carry the built-in provider id, which remains
// enough to recover the underlying plugin id for marketplace-backed tools.
if (value.type && value.type !== CollectionType.builtIn)
return null
const providerNameSegments = value.provider_name.split('/')
if (providerNameSegments.length !== 3)
return null
return providerNameSegments.slice(0, 2).join('/')
}, [
areToolProvidersSettled,
currentProvider,
value?.plugin_id,
value?.provider_name,
value?.type,
])
// Plugin info check
const { inMarketPlace, manifest, pluginID } = usePluginInstalledCheck({
providerPluginId,
enabled: !!value?.provider_name
&& (!currentProvider || !currentTool)
&& (currentProvider !== undefined || areToolProvidersSettled || !!value?.plugin_id),
})
// Tool settings and params
const currentToolSettings = useMemo(() => {
@ -120,6 +163,7 @@ export const useToolSelectorState = ({
return {
provider_name: tool.provider_id,
provider_show_name: tool.provider_name,
plugin_id: tool.plugin_id,
tool_name: tool.tool_name,
tool_label: tool.tool_label,
tool_description: tool.tool_description,

View File

@ -79,6 +79,7 @@ export type PluginDefaultValue = ToolDefaultValue | DataSourceDefaultValue | Tri
export type ToolValue = {
provider_name: string
provider_show_name?: string
plugin_id?: string
tool_name: string
tool_label: string
tool_description?: string