diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts index 81f3e4fccce..ef83ac57850 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSkillModal.test.ts @@ -117,6 +117,7 @@ function renderModal({ | 'onRemove' | 'outdated' | 'onUseLatestVersion' + | 'getDefaultName' > = {}) { const renderComponent = createComponentRenderer(AgentSkillModal, { global: { @@ -725,6 +726,23 @@ describe('AgentSkillModal', () => { expect(getByTestId('agent-skill-picker')).toBeVisible(); }); + it('asks for a default name where "available to" puts the new skill', async () => { + listAttachableSkills.mockResolvedValue([]); + const getDefaultName = vi.fn().mockResolvedValue('New skill 3'); + const { getByTestId } = renderModal({ + hubSkillsEnabled: true, + onAttach: vi.fn(), + getDefaultName, + }); + + // "Create" from the "This agent" tab makes the skill available to this agent only. + await fireEvent.click(getByTestId('agent-skill-picker-create-this-agent')); + await fireEvent.click(getByTestId('agent-skill-add-manually')); + + expect(getDefaultName).toHaveBeenCalledWith('agent'); + await waitFor(() => expect(getTitleInput(document.body)).toHaveValue('New skill 3')); + }); + it('opens a read-only preview from a row click and adds the skill from there', async () => { listAttachableSkills.mockResolvedValue([projectSkill, instanceSkill]); getHubSkill.mockResolvedValue( diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts index 57b0bac1ae8..debd6732cfa 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentCapabilitiesActions.test.ts @@ -8,8 +8,9 @@ import { useUIStore } from '@/app/stores/ui.store'; import { useSettingsStore } from '@n8n/stores/settings.store'; import { AGENT_TOOLS_MODAL_KEY, AGENT_SKILL_MODAL_KEY } from '../constants'; import { useAgentCapabilitiesActions } from '../composables/useAgentCapabilitiesActions'; -import { createAgentSkill, getHubSkill } from '../composables/useAgentApi'; -import type { SkillDetail, SkillListItem } from '@n8n/api-types'; +import { createAgentSkill, getHubSkill, getNewSkillName } from '../composables/useAgentApi'; +import { SKILL_NAME_TAKEN_MESSAGE, type SkillDetail, type SkillListItem } from '@n8n/api-types'; +import { ResponseError } from '@n8n/rest-api-client'; import type { AgentJsonConfig, AgentJsonMcpServerConfig, @@ -37,6 +38,7 @@ vi.mock('@n8n/composables/useToast', () => ({ vi.mock('../composables/useAgentApi', () => ({ createAgentSkill: vi.fn(), getHubSkill: vi.fn(), + getNewSkillName: vi.fn(), })); const { hasScopeSpy } = vi.hoisted(() => ({ @@ -177,11 +179,12 @@ type SkillModalData = { onUseLatestVersion?: (skillId: string) => void; getAttachedSkills?: () => Array<{ id: string; name: string }>; canCreateInstanceSkills?: boolean; + getDefaultName?: (availableTo: string) => Promise; onConfirm: (payload: { id?: string; skill: AgentSkill; availableTo?: 'project' | 'agent' | 'instance'; - }) => void; + }) => unknown; onAttach?: (skill: SkillListItem) => Promise; onRemove?: (skillId: string) => void; }; @@ -244,6 +247,12 @@ describe('useAgentCapabilitiesActions — localSkills host seam', () => { const persisted = new Promise((resolve) => { releasePersist = resolve; }); + vi.mocked(createAgentSkill).mockResolvedValue({ + id: 'skill_new', + skill: { ...triage, name: 'New Skill' }, + skillHash: 'hash-new', + versionId: 'v2', + }); const { uiStore, actions } = setup({ ensureAgentPersisted: async () => await persisted }); actions.onOpenAddSkillModal(); @@ -730,6 +739,50 @@ describe('useAgentCapabilitiesActions — skills hub', () => { expect(hasScopeSpy).toHaveBeenCalledWith('skill:create'); }); + it.each([ + ['project', { scope: 'project', projectId: 'project-1' }], + ['agent', { scope: 'agent', agentId: 'agent-1' }], + ['instance', { scope: 'instance' }], + ])( + 'asks the server for a default name where "available to" %s puts the skill', + async (availableTo, target) => { + vi.mocked(getNewSkillName).mockResolvedValue('New skill 3'); + const { modalData } = setupHub(); + + await expect(modalData.getDefaultName?.(availableTo)).resolves.toBe('New skill 3'); + expect(getNewSkillName).toHaveBeenCalledWith(expect.anything(), { + ...target, + name: 'agents.builder.skills.defaultName', + }); + }, + ); + + it('keeps the modal open with the error on the name when the name is taken', async () => { + vi.mocked(createAgentSkill).mockRejectedValue( + new ResponseError(SKILL_NAME_TAKEN_MESSAGE, { httpStatusCode: 409 }), + ); + const { modalData } = setupHub(); + + const result = await modalData.onConfirm({ skill: sqlHelper, availableTo: 'project' }); + + expect(result).toEqual({ + saved: false, + nameError: 'settings.context.skills.error.nameTaken', + }); + expect(showErrorSpy).not.toHaveBeenCalled(); + }); + + it('keeps the modal open and reports any other failed create', async () => { + const error = new Error('Network down'); + vi.mocked(createAgentSkill).mockRejectedValue(error); + const { modalData } = setupHub(); + + const result = await modalData.onConfirm({ skill: sqlHelper, availableTo: 'project' }); + + expect(result).toEqual({ saved: false }); + expect(showErrorSpy).toHaveBeenCalledWith(error, 'agents.builder.skills.create.error'); + }); + it('keeps the picker off and creates the skill on the agent when the skills hub is off', async () => { vi.mocked(createAgentSkill).mockResolvedValue({ id: 'sql', @@ -740,6 +793,8 @@ describe('useAgentCapabilitiesActions — skills hub', () => { const { modalData, actions, uiStore } = setupHub({ skillsHubEnabled: false }); expect(modalData.hubSkillsEnabled).toBe(false); + // Without the hub, names are only unique per agent, so the modal numbers them itself. + expect(modalData.getDefaultName).toBeUndefined(); modalData.onConfirm({ skill: sqlHelper, availableTo: 'project' }); await flushPromises(); expect(createAgentSkill).toHaveBeenCalledWith( diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue index a0474f86827..7391363d15c 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentSkillModal.vue @@ -76,8 +76,9 @@ export type AgentSkillModalData = { scopeOptions?: AgentSkillModalScopeOption[]; initialScope?: string; /** - * Settings only: a default name that no skill in the scope has yet, as new workflows - * get one. Called with the chosen scope value when the user starts a skill by hand. + * A default name that no skill in the scope has yet, as new workflows get one. Called + * when the user starts a skill by hand, with the Settings scope value or, in the agent + * builder, the "available to" value. */ getDefaultName?: (scope: string) => Promise; /** Locks the scope field and explains why in a tooltip. */ @@ -162,13 +163,13 @@ async function refreshDefaultName() { const current = skill.value.name; if (current !== getDefaultSkillName() && current !== serverDefaultName.value) return; const request = ++latestNameRequest; - const requestedScope = scope.value; + const requestedScope = nameScope.value; defaultNameLoading.value = true; try { const name = await props.data.getDefaultName(requestedScope); // Only the newest answer for the scope still selected counts, and only while the // user has not typed a name of their own. - if (request !== latestNameRequest || scope.value !== requestedScope) return; + if (request !== latestNameRequest || nameScope.value !== requestedScope) return; if (skill.value.name !== current) return; serverDefaultName.value = name; skill.value = { ...skill.value, name }; @@ -184,10 +185,6 @@ function onAddManually() { void refreshDefaultName(); } -// A new scope can hold other names, so a default name follows the scope. -watch(scope, () => { - if (step.value === 'manual') void refreshDefaultName(); -}); const showScope = computed(() => !!props.data.scopeOptions?.length && step.value !== 'preview'); const canChangeScope = computed( () => showScope.value && !props.data.scopeLockedReason && !props.data.readOnly, @@ -196,6 +193,15 @@ const selectedScopeIcon = computed( () => props.data.scopeOptions?.find((option) => option.value === scope.value)?.icon, ); const availableTo = ref('project'); +/** + * Where the new skill goes, for its default name: the scope field in Settings, or + * "available to" in the agent builder. `getDefaultName` gets this value. + */ +const nameScope = computed(() => (showScope.value ? scope.value : availableTo.value)); +// A new scope can hold other names, so a default name follows the scope. +watch(nameScope, () => { + if (step.value === 'manual') void refreshDefaultName(); +}); const librarySkills = ref([]); const libraryLoading = ref(false); diff --git a/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts b/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts index 292f603fe0e..577595a57bf 100644 --- a/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts +++ b/packages/frontend/editor-ui/src/features/agents/composables/useAgentApi.ts @@ -37,6 +37,8 @@ import type { SkillDetail, SkillListItem, SkillListResponse, + SkillNewNameQueryDto, + SkillNewNameResponse, VectorStoreTestResult, } from '@n8n/api-types'; import { getFullApiResponse, makeRestApiRequest, request } from '@n8n/rest-api-client'; @@ -755,6 +757,20 @@ export const getHubSkill = async ( return await makeRestApiRequest(context, 'GET', `/skills/${skillId}`); }; +/** A name like `name` that no skill in the scope has yet, such as "New skill 2". */ +export const getNewSkillName = async ( + context: IRestApiContext, + query: SkillNewNameQueryDto, +): Promise => { + const { name } = await makeRestApiRequest( + context, + 'GET', + '/skills/new-name', + query, + ); + return name; +}; + export const getAgentBackgroundJobs = async ( context: IRestApiContext, projectId: string, diff --git a/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts b/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts index 271fc63ebd3..6eb61b2e61e 100644 --- a/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts +++ b/packages/frontend/editor-ui/src/features/agents/composables/useAgentCapabilitiesActions.ts @@ -10,11 +10,12 @@ import { type AgentSkillAvailability, type SkillDetail, type SkillListItem, + type SkillNewNameQueryDto, } from '@n8n/api-types'; import { useUIStore } from '@/app/stores/ui.store'; import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; import { AI_MCP_TOOL_NODE_TYPE } from '@/app/constants/nodeTypes'; -import { createAgentSkill, getHubSkill } from './useAgentApi'; +import { createAgentSkill, getHubSkill, getNewSkillName } from './useAgentApi'; import { useAgentProjectBreadcrumb } from './useAgentProjectBreadcrumb'; import { mcpServerToNode } from './useMcpServerAdapter'; import { @@ -22,8 +23,9 @@ import { AGENT_TOOL_CONFIG_MODAL_KEY, AGENT_SKILL_MODAL_KEY, } from '../constants'; -import { normalizeAgentSkillForSave } from '../utils/agentSkill'; +import { isSkillNameTakenError, normalizeAgentSkillForSave } from '../utils/agentSkill'; import type { ToolOpenTarget, ToolPickerMode } from '../components/AgentCapabilitiesSection.types'; +import type { AgentSkillConfirmResult } from '../components/AgentSkillModal.vue'; import type { AgentResource, AgentJsonConfig, @@ -410,6 +412,17 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep * Authoring-time mirror of the backend's `assertSkillNameIsUnique` for * local-skill hosts. The modal validates against `existingSkillNames` before closing */ + /** The scope of the new-name lookup for an "available to" choice. */ + function newNameTarget( + availableTo: string, + targetProjectId: string, + targetAgentId: string, + ): Pick { + if (availableTo === 'instance') return { scope: 'instance' }; + if (availableTo === 'agent') return { scope: 'agent', agentId: targetAgentId }; + return { scope: 'project', projectId: targetProjectId }; + } + function hasDuplicateSkillName(name: string, excludeId?: string): boolean { const key = skillNameKey(name); return appliedSkillNames(excludeId).some((existing) => skillNameKey(existing) === key); @@ -484,6 +497,17 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep appliedSkills.value.map(({ id, skill }) => ({ id, name: skill.name })), projectLabel: projectName.value ?? undefined, canCreateInstanceSkills: rbacStore.hasScope('skill:create'), + // The hub refuses a name that the skill's scope already has, so the default + // name comes from the server, for the "available to" choice. + ...(hubSkillsEnabled.value + ? { + getDefaultName: async (availableTo: string) => + await getNewSkillName(rootStore.restApiContext, { + ...newNameTarget(availableTo, targetProjectId, targetAgentId), + name: locale.baseText('agents.builder.skills.defaultName'), + }), + } + : {}), onRemove: (skillId: string) => { if (!isCurrentTarget()) return; onRemoveSkill(skillId); @@ -493,59 +517,63 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep isCurrentTarget, async () => await getHubSkill(rootStore.restApiContext, librarySkill.id), ), - onConfirm: ({ + // The modal stays open until the skill is created, and stays open when the create + // fails, so the user keeps what they typed. A taken name shows on the name field. + onConfirm: async ({ skill, availableTo, }: { id?: string; skill: AgentSkill; availableTo?: AgentSkillAvailability; - }) => { + }): Promise => { if (localSkills) { - if (!isCurrentTarget()) return; + if (!isCurrentTarget()) return undefined; const sanitizedSkill = normalizeAgentSkillForSave(skill); if (hasDuplicateSkillName(sanitizedSkill.name)) { showDuplicateSkillNameError(sanitizedSkill.name); - return; + return undefined; } // The host mints the skill id and writes body + ref together. localSkills.createSkill(sanitizedSkill); - return; + return undefined; } + const sanitizedSkill = normalizeAgentSkillForSave(skill); + try { + await beforeAgentMutation?.(); + } catch { + // The host owns the autosave error message. Do not also report + // this as a skill-creation failure. + return { saved: false }; + } + if (!isCurrentTarget()) return undefined; + let result: Awaited>; + try { + await ensureAgentPersisted?.(); + if (!isCurrentTarget()) return undefined; + result = await createAgentSkill( + rootStore.restApiContext, + targetProjectId, + targetAgentId, + sanitizedSkill, + hubSkillsEnabled.value ? availableTo : undefined, + ); + } catch (error) { + if (isSkillNameTakenError(error)) { + return { + saved: false, + nameError: locale.baseText('settings.context.skills.error.nameTaken'), + }; + } + showError(error, locale.baseText('agents.builder.skills.create.error')); + return { saved: false }; + } + + // The skill exists now, so the modal can close while the agent catches up. void (async () => { - const sanitizedSkill = normalizeAgentSkillForSave(skill); - let created: AgentSkill; - let skillHash: string; - let versionId: string | null; - let skillId: string; - try { - await beforeAgentMutation?.(); - } catch { - // The host owns the autosave error message. Do not also report - // this as a skill-creation failure. - return; - } - if (!isCurrentTarget()) return; - try { - await ensureAgentPersisted?.(); - if (!isCurrentTarget()) return; - const result = await createAgentSkill( - rootStore.restApiContext, - targetProjectId, - targetAgentId, - sanitizedSkill, - hubSkillsEnabled.value ? availableTo : undefined, - ); - skillId = result.id; - created = result.skill; - skillHash = result.skillHash; - versionId = result.versionId; - } catch (error) { - showError(error, locale.baseText('agents.builder.skills.create.error')); - return; - } + const { id: skillId, skill: created, skillHash, versionId } = result; if (!isCurrentTarget() || agent.value?.id !== targetAgentId) return; agent.value = { ...agent.value, @@ -577,6 +605,7 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep }; } })(); + return undefined; }, }, }); diff --git a/packages/frontend/editor-ui/src/features/agents/utils/agentSkill.ts b/packages/frontend/editor-ui/src/features/agents/utils/agentSkill.ts index 0a8fb21d086..b73fd715da7 100644 --- a/packages/frontend/editor-ui/src/features/agents/utils/agentSkill.ts +++ b/packages/frontend/editor-ui/src/features/agents/utils/agentSkill.ts @@ -1,5 +1,17 @@ +import { SKILL_NAME_TAKEN_MESSAGE } from '@n8n/api-types'; +import { ResponseError } from '@n8n/rest-api-client'; + import type { AgentSkill } from '../types'; +/** Whether a save failed because the skill's scope already has a name like it. */ +export function isSkillNameTakenError(error: unknown): boolean { + return ( + error instanceof ResponseError && + error.httpStatusCode === 409 && + error.message === SKILL_NAME_TAKEN_MESSAGE + ); +} + /** Skills drop `allowedTools`: the editor does not show it and the runtime does not get it. */ export function normalizeAgentSkillForSave(skill: AgentSkill): AgentSkill { const { allowedTools: _allowedTools, ...rest } = skill; diff --git a/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue b/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue index bcb3386c8c3..26c1c94d7b2 100644 --- a/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue +++ b/packages/frontend/editor-ui/src/features/settings/context/views/SettingsSkillsView.vue @@ -2,14 +2,8 @@ import { computed, onMounted, ref } from 'vue'; import { useRoute, useRouter } from 'vue-router'; import { useDebounceFn } from '@vueuse/core'; -import { - SKILL_NAME_TAKEN_MESSAGE, - type AgentSkill, - type SkillListItem, - type SkillScope, -} from '@n8n/api-types'; +import type { AgentSkill, SkillListItem, SkillScope } from '@n8n/api-types'; import { useI18n } from '@n8n/i18n'; -import { ResponseError } from '@n8n/rest-api-client'; import { useToast } from '@n8n/composables/useToast'; import { getDebounceTime } from '@n8n/composables/useDebounce'; import { @@ -31,6 +25,7 @@ import { DEBOUNCE_TIME, MODAL_CONFIRM, VIEWS } from '@/app/constants'; import { useUIStore } from '@/app/stores/ui.store'; import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; import { AGENT_SKILL_MODAL_KEY } from '@/features/agents/constants'; +import { isSkillNameTakenError } from '@/features/agents/utils/agentSkill'; import type { AgentSkillConfirmFailure, AgentSkillModalData, @@ -221,11 +216,7 @@ function confirmIfHidden(skillId: string, key: 'created' | 'saved') { * shows on the name field, any other error in a toast. */ function saveFailed(error: unknown): AgentSkillConfirmFailure { - if ( - error instanceof ResponseError && - error.httpStatusCode === 409 && - error.message === SKILL_NAME_TAKEN_MESSAGE - ) { + if (isSkillNameTakenError(error)) { return { saved: false, nameError: i18n.baseText('settings.context.skills.error.nameTaken') }; } showError(error, i18n.baseText('settings.context.skills.error.save'));