fix(ai-builder): Surface skill save validation errors and stop reverting skill renames (#36055)

This commit is contained in:
Anne Aguirre
2026-08-13 05:39:18 +00:00
committed by GitHub
parent 4fe94d8f5e
commit b08cde668f
4 changed files with 90 additions and 11 deletions
@@ -5,7 +5,10 @@ import { mockedStore } from '@/__tests__/utils';
import { useUIStore } from '@/app/stores/ui.store';
import { fireEvent } from '@testing-library/vue';
import { defineComponent, h, onMounted, watch } from 'vue';
import { AGENT_SKILL_REFERENCE_MAX_COUNT } from '@n8n/api-types';
import {
AGENT_SKILL_INSTRUCTIONS_MAX_LENGTH,
AGENT_SKILL_REFERENCE_MAX_COUNT,
} from '@n8n/api-types';
import AgentSkillModal from '../components/AgentSkillModal.vue';
import type { AgentSkill } from '../types';
@@ -20,6 +23,11 @@ vi.mock('../composables/useAgentApi', () => ({
createAgentSkill: (...args: unknown[]) => apiCreateSpy(...args),
}));
const { showMessage } = vi.hoisted(() => ({ showMessage: vi.fn() }));
vi.mock('@n8n/composables/useToast', () => ({
useToast: () => ({ showMessage }),
}));
const ModalStub = defineComponent({
props: ['name', 'customClass', 'width'],
template: `
@@ -42,7 +50,9 @@ const SkillViewerStub = defineComponent({
return Boolean(
props.skill?.name?.trim() &&
props.skill?.description?.trim() &&
props.skill?.instructions?.trim(),
props.skill?.instructions?.trim() &&
new TextEncoder().encode(props.skill.instructions).byteLength <=
AGENT_SKILL_INSTRUCTIONS_MAX_LENGTH,
);
}
onMounted(() => emit('update:valid', computeValid()));
@@ -139,6 +149,35 @@ describe('AgentSkillModal', () => {
expect(onConfirm).not.toHaveBeenCalled();
expect(uiStore.closeModal).not.toHaveBeenCalled();
expect(showMessage).toHaveBeenCalledWith({
title: 'agents.builder.skills.saveError',
message: 'agents.builder.skills.validation.descriptionRequired',
type: 'error',
});
});
it('explains why overlong instructions cannot be saved', async () => {
const onConfirm = vi.fn();
const { container } = renderModal({
onConfirm,
skill: {
name: 'Research',
description: 'Use for research',
instructions: 'x'.repeat(AGENT_SKILL_INSTRUCTIONS_MAX_LENGTH + 1),
},
});
await fireEvent.click(
container.querySelector('[data-testid="agent-skill-create-save"]') as Element,
);
expect(onConfirm).not.toHaveBeenCalled();
expect(uiStore.closeModal).not.toHaveBeenCalled();
expect(showMessage).toHaveBeenCalledWith({
title: 'agents.builder.skills.saveError',
message: 'agents.builder.skills.validation.instructionsMaxLength',
type: 'error',
});
});
it('adds and removes references from the file navigation', async () => {
@@ -1,10 +1,14 @@
<script setup lang="ts">
import { computed, ref } from 'vue';
import { AGENT_SKILL_REFERENCE_MAX_COUNT } from '@n8n/api-types';
import {
AGENT_SKILL_INSTRUCTIONS_MAX_LENGTH,
AGENT_SKILL_REFERENCE_MAX_COUNT,
} from '@n8n/api-types';
import { N8nButton, N8nCallout, N8nHeading, N8nIcon } from '@n8n/design-system';
import { useI18n, type BaseTextKey } from '@n8n/i18n';
import Modal from '@/app/components/Modal.vue';
import { useToast } from '@n8n/composables/useToast';
import { useUIStore } from '@/app/stores/ui.store';
import { useAgentTelemetry } from '../composables/useAgentTelemetry';
import type { AgentSkill } from '../types';
@@ -37,6 +41,7 @@ const props = defineProps<{
const i18n = useI18n();
const uiStore = useUIStore();
const agentTelemetry = useAgentTelemetry();
const { showMessage } = useToast();
const skill = ref<AgentSkill>(
normalizeSkill({
@@ -84,6 +89,13 @@ const validationErrors = computed<Partial<Record<keyof AgentSkill, string>>>(()
if (!instructions) {
errors.instructions = i18n.baseText('agents.builder.skills.validation.instructionsRequired');
} else if (
new TextEncoder().encode(skill.value.instructions).byteLength >
AGENT_SKILL_INSTRUCTIONS_MAX_LENGTH
) {
errors.instructions = i18n.baseText('agents.builder.skills.validation.instructionsMaxLength', {
interpolate: { max: String(AGENT_SKILL_INSTRUCTIONS_MAX_LENGTH) },
});
}
if (skill.value.references?.some((reference) => !reference.content.trim())) {
errors.references = i18n.baseText('agents.builder.skills.references.invalidSummary');
@@ -168,7 +180,19 @@ function closeModal() {
function onSave() {
submitted.value = true;
if (!canSave.value) return;
if (!canSave.value) {
const message =
validationErrors.value.name ??
validationErrors.value.description ??
validationErrors.value.instructions ??
validationErrors.value.references;
showMessage({
title: i18n.baseText('agents.builder.skills.saveError'),
...(message ? { message } : {}),
type: 'error',
});
return;
}
const payload = normalizeSkill({
name: skill.value.name.trim(),
@@ -156,7 +156,7 @@ describe('useAgentCapabilitiesActions', () => {
it('saves a skill-modal confirm for the agent it was opened on', () => {
const skill: AgentSkill = { name: 'PR Reviewer', description: '', instructions: 'Review.' };
const { actions, scheduleSkillSave, agent } = makeActions({
const { actions, scheduleConfigUpdate, scheduleSkillSave, agent } = makeActions({
skills: [{ type: 'skill', id: 's1' }],
} as Partial<AgentJsonConfig>);
agent.value = { id: 'agent-1', skills: { s1: skill } } as unknown as AgentResource;
@@ -171,6 +171,28 @@ describe('useAgentCapabilitiesActions', () => {
skillId: 's1',
skill: expect.objectContaining({ instructions: 'Edited.' }),
});
expect(scheduleConfigUpdate).not.toHaveBeenCalled();
});
it('persists a skill rename without scheduling a config save', () => {
const skill: AgentSkill = { name: 'PR Reviewer', description: '', instructions: 'Review.' };
const { actions, scheduleConfigUpdate, scheduleSkillSave, agent } = makeActions({
skills: [{ type: 'skill', id: 's1' }],
} as Partial<AgentJsonConfig>);
agent.value = { id: 'agent-1', skills: { s1: skill } } as unknown as AgentResource;
actions.onOpenSkillFromList('s1');
const modalData = openModalWithData.mock.calls[0][0] as {
data: { onConfirm: (payload: { id?: string; skill: AgentSkill }) => void };
};
modalData.data.onConfirm({ id: 's1', skill: { ...skill, name: 'Renamed skill' } });
expect(scheduleSkillSave).toHaveBeenCalledWith({
skillId: 's1',
skill: expect.objectContaining({ name: 'Renamed skill' }),
});
expect(scheduleConfigUpdate).not.toHaveBeenCalled();
expect(agent.value.skills?.s1?.name).toBe('Renamed skill');
});
it('drops the tool ref from the config when onRemoveTool removes it', () => {
@@ -306,12 +306,6 @@ export function useAgentCapabilitiesActions(deps: UseAgentCapabilitiesActionsDep
[skillId]: sanitizedSkill,
},
};
const nextSkills = [...(localConfig.value?.skills ?? [])];
const skillRefIndex = nextSkills.findIndex((skillRef) => skillRef.id === id);
if (skillRefIndex !== -1) {
nextSkills[skillRefIndex] = { type: 'skill', id: skillId };
scheduleConfigUpdate({ skills: nextSkills });
}
scheduleSkillSave({ skillId, skill: sanitizedSkill });
},
},