From 21f5097bfefa33bb431c198db26fc53c5db8eb55 Mon Sep 17 00:00:00 2001 From: Robin Braumann <50590409+bjorger@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:05:53 +0000 Subject: [PATCH] refactor(core): Centralize agent draft model/credential validation (no-changelog) (#34775) Co-authored-by: Cursor --- .../agent-json-config.schema.test.ts | 34 +++++++++++++ .../src/agents/agent-config-lifecycle.ts | 15 ++++++ .../agents/agent-config-validation.schema.ts | 1 + .../src/agents/agent-json-config.schema.ts | 22 ++++++-- packages/@n8n/api-types/src/agents/dto.ts | 3 +- packages/@n8n/api-types/src/agents/index.ts | 1 + .../src/agents/inline-agent-config.schema.ts | 6 +-- .../src/agents/sanitize-agent-json-config.ts | 8 +-- .../agent-integrations.controller.test.ts | 42 +++++++++++++++ .../agent-validation.service.test.ts | 51 +++++++++++++++++++ .../agents-builder-tools.service.test.ts | 26 ++++++++-- .../agent-integration-persistence.service.ts | 5 +- .../agents/agent-integrations.controller.ts | 3 +- .../agents/agent-model-catalog.service.ts | 2 +- .../modules/agents/agent-publish.service.ts | 3 +- .../agents/agent-validation.service.ts | 36 +++++++++++-- .../builder/agents-builder-tools.service.ts | 37 ++++++++++---- .../__tests__/resolve-llm.tool.test.ts | 2 +- .../builder/interactive/resolve-llm.tool.ts | 2 +- .../builder/prompts/config-rules.prompt.ts | 4 +- ...sanitize-unknown-agent-credentials.test.ts | 29 ++++++++++- .../sanitize-unknown-agent-credentials.ts | 26 +++++++++- .../interactive => }/llm-provider-defaults.ts | 0 .../__tests__/AgentChannelModal.test.ts | 34 +++++++++++-- .../agents/components/AgentChannelModal.vue | 6 +-- 25 files changed, 346 insertions(+), 52 deletions(-) create mode 100644 packages/@n8n/api-types/src/agents/agent-config-lifecycle.ts rename packages/cli/src/modules/agents/{builder/interactive => }/llm-provider-defaults.ts (100%) diff --git a/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts b/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts index 37b36e6a9e5..6d02050c2a8 100644 --- a/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts +++ b/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts @@ -508,3 +508,37 @@ describe('AgentJsonConfigSchema — skills', () => { } }); }); + +describe('AgentJsonConfigSchema — model/credential coupling', () => { + it('rejects a credential without a model', () => { + const result = AgentJsonConfigSchema.safeParse({ + ...minimalConfig, + model: '', + credential: 'cred-id', + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.errors[0].path).toEqual(['credential']); + } + }); + + it('accepts a model without a credential', () => { + const result = AgentJsonConfigSchema.safeParse({ + ...minimalConfig, + credential: undefined, + }); + + expect(result.success).toBe(true); + }); + + it('accepts a fully cleared draft', () => { + const result = AgentJsonConfigSchema.safeParse({ + ...minimalConfig, + model: '', + credential: '', + }); + + expect(result.success).toBe(true); + }); +}); diff --git a/packages/@n8n/api-types/src/agents/agent-config-lifecycle.ts b/packages/@n8n/api-types/src/agents/agent-config-lifecycle.ts new file mode 100644 index 00000000000..7f0a55ad4da --- /dev/null +++ b/packages/@n8n/api-types/src/agents/agent-config-lifecycle.ts @@ -0,0 +1,15 @@ +/** + * Draft-lifecycle predicates. A capability is a "draft" while its setup is + * pending: the agent config until a model is chosen (`model: ""`), an + * integration entry until a credential is connected (`credentialId: ""`). + */ + +/** True while no model has been chosen yet (setup pending). */ +export function isDraftAgentConfig(config: { model?: string } | null | undefined): boolean { + return typeof config?.model !== 'string' || config.model.trim() === ''; +} + +/** True while no credential is connected yet (setup pending). */ +export function isDraftIntegration(integration: { credentialId: string }): boolean { + return integration.credentialId.trim() === ''; +} diff --git a/packages/@n8n/api-types/src/agents/agent-config-validation.schema.ts b/packages/@n8n/api-types/src/agents/agent-config-validation.schema.ts index 0112d10d6a8..b455c74f1fd 100644 --- a/packages/@n8n/api-types/src/agents/agent-config-validation.schema.ts +++ b/packages/@n8n/api-types/src/agents/agent-config-validation.schema.ts @@ -14,6 +14,7 @@ export const agentCapabilityKindSchema = z.enum([ 'skill', 'task', 'subAgent', + 'vectorStore', ]); export type AgentCapabilityKind = z.infer; diff --git a/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts b/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts index fa47c7b52ac..09b9371cfd9 100644 --- a/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts +++ b/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts @@ -1,5 +1,6 @@ import { z, type ZodError } from 'zod'; +import { isDraftAgentConfig } from './agent-config-lifecycle'; import { AgentIntegrationConfigSchema } from './agent-integration.schema'; /** * Regex for valid custom tool ids. Shared with the backend service layer @@ -401,7 +402,12 @@ const AgentJsonToolConfigSchema = z.discriminatedUnion('type', [ NodeToolJsonConfigSchema, ]); -export const AgentJsonConfigSchema = z.object({ +/** + * Unrefined agent config object shape. Use for schema derivation only + * (`.extend`, `.pick`, `.partial`, `.shape`) — validate with + * {@link AgentJsonConfigSchema} instead. + */ +export const AgentJsonConfigBaseSchema = z.object({ name: z.string().min(1).max(128), model: DraftAgentModelSchema, credential: z.string().optional(), @@ -479,15 +485,23 @@ export const AgentJsonConfigSchema = z.object({ .optional(), }); -export const RunnableAgentJsonConfigSchema = AgentJsonConfigSchema.extend({ +export const AgentJsonConfigSchema = AgentJsonConfigBaseSchema.superRefine((config, ctx) => { + if (config.credential?.trim() && isDraftAgentConfig(config)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['credential'], + message: 'A credential requires a model to be set', + }); + } +}); + +export const RunnableAgentJsonConfigSchema = AgentJsonConfigBaseSchema.extend({ model: AgentModelSchema, credential: z.string().refine((value) => value.trim().length > 0, { message: 'Credential is required', }), }); -export const AgentJsonConfigPartialSchema = AgentJsonConfigSchema.partial(); - export type AgentJsonConfig = z.infer; export type RunnableAgentJsonConfig = z.infer; export type AgentJsonToolConfig = z.infer; diff --git a/packages/@n8n/api-types/src/agents/dto.ts b/packages/@n8n/api-types/src/agents/dto.ts index 6c27d10ba14..7b8c3e709d3 100644 --- a/packages/@n8n/api-types/src/agents/dto.ts +++ b/packages/@n8n/api-types/src/agents/dto.ts @@ -142,7 +142,8 @@ export class AgentChatResumeDto extends Z.class({ export class AgentDisconnectIntegrationDto extends Z.class({ type: z.string().min(1), - credentialId: z.string().min(1), + // Empty string targets a draft integration entry (`credentialId: ''`). + credentialId: z.string(), }) {} export class PublishAgentDto extends Z.class({ diff --git a/packages/@n8n/api-types/src/agents/index.ts b/packages/@n8n/api-types/src/agents/index.ts index 4ae4b78dfdb..9bc33bc48dd 100644 --- a/packages/@n8n/api-types/src/agents/index.ts +++ b/packages/@n8n/api-types/src/agents/index.ts @@ -1,3 +1,4 @@ +export * from './agent-config-lifecycle'; export * from './agent-config-validation.schema'; export * from './agent-files.constants'; export * from './agent-integration.schema'; diff --git a/packages/@n8n/api-types/src/agents/inline-agent-config.schema.ts b/packages/@n8n/api-types/src/agents/inline-agent-config.schema.ts index 200f501ddba..2803218a1f2 100644 --- a/packages/@n8n/api-types/src/agents/inline-agent-config.schema.ts +++ b/packages/@n8n/api-types/src/agents/inline-agent-config.schema.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import { - AgentJsonConfigSchema, + AgentJsonConfigBaseSchema, AgentModelSchema, NodeToolJsonConfigSchema, WorkflowToolJsonConfigSchema, @@ -43,7 +43,7 @@ const InlineAgentSkillBodiesSchema = z.record(InlineSkillIdSchema, agentSkillSch * only available on saved agents; runtime defaults apply. Skills are supported * as refs here, with their bodies in the sibling `skills` record. */ -export const InlineAgentJsonConfigSchema = AgentJsonConfigSchema.pick({ +export const InlineAgentJsonConfigSchema = AgentJsonConfigBaseSchema.pick({ name: true, model: true, credential: true, @@ -55,7 +55,7 @@ export const InlineAgentJsonConfigSchema = AgentJsonConfigSchema.pick({ // Approval suspends the run for a human, which workflow executions // don't support — same reason the tool variants above omit // `requireApproval`. - mcpServers: AgentJsonConfigSchema.shape.mcpServers.refine( + mcpServers: AgentJsonConfigBaseSchema.shape.mcpServers.refine( (servers) => (servers ?? []).every((server) => server.approval === undefined), { message: 'MCP tool approval is not available for inline agents' }, ), diff --git a/packages/@n8n/api-types/src/agents/sanitize-agent-json-config.ts b/packages/@n8n/api-types/src/agents/sanitize-agent-json-config.ts index 3402133632c..1f8c59494b4 100644 --- a/packages/@n8n/api-types/src/agents/sanitize-agent-json-config.ts +++ b/packages/@n8n/api-types/src/agents/sanitize-agent-json-config.ts @@ -1,6 +1,6 @@ import { z, type ZodDiscriminatedUnionOption } from 'zod'; -import { AgentJsonConfigSchema } from './agent-json-config.schema'; +import { AgentJsonConfigBaseSchema } from './agent-json-config.schema'; import { agentSkillSchema } from './agent-skill.schema'; const TYPED_ARRAY_CONFIG_KEYS = ['integrations', 'tools', 'skills', 'tasks'] as const; @@ -190,7 +190,7 @@ function stripUnknownSchemaFields(value: unknown, schema: z.ZodTypeAny): unknown /** * Strip legacy or unsupported typed entries from agent JSON config before strict - * Zod validation. Unknown top-level keys are dropped from `AgentJsonConfigSchema`. + * Zod validation. Unknown top-level keys are dropped from `AgentJsonConfigBaseSchema`. * This intentionally cleans unknown fields gracefully, so older persisted configs * and generated drafts can move forward as the schema evolves. * @@ -202,12 +202,12 @@ export function sanitizeAgentJsonConfig(raw: unknown): unknown { return raw; } - const sanitized = stripUnknownSchemaFields(raw, AgentJsonConfigSchema); + const sanitized = stripUnknownSchemaFields(raw, AgentJsonConfigBaseSchema); if (!isRecord(sanitized)) return sanitized; for (const key of TYPED_ARRAY_CONFIG_KEYS) { if (key in sanitized) { - const schema = getArrayElementSchema(AgentJsonConfigSchema.shape[key]); + const schema = getArrayElementSchema(AgentJsonConfigBaseSchema.shape[key]); if (schema === undefined) continue; sanitized[key] = filterUnsupportedTypedEntries( diff --git a/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts b/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts index 025e9fc5d57..cc457415a97 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts @@ -635,6 +635,48 @@ describe('AgentIntegrationsController integration credentials', () => { ); }); + it('disconnects a draft integration entry with an empty credentialId', async () => { + const agentRepository = mock(); + const agent = { + id: 'agent-1', + projectId: 'project-1', + integrations: [{ type: 'slack', credentialId: '' }], + }; + agentRepository.findByIdAndProjectId.mockResolvedValue(agent as never); + + const chatIntegrationService = mock(); + const agentIntegrationPersistenceService = mock(); + const { controller } = makeController({ + agentRepository, + chatIntegrationService, + agentIntegrationPersistenceService, + }); + + await expect( + controller.disconnectIntegration( + { + params: { projectId: 'project-1' }, + user: { id: 'user-1' }, + body: { type: 'slack', credentialId: '' }, + } as never, + undefined as never, + 'agent-1', + { type: 'slack', credentialId: '' }, + ), + ).resolves.toEqual({ status: 'disconnected' }); + + expect(chatIntegrationService.disconnectChannel).toHaveBeenCalledWith('agent-1', { + type: 'slack', + credentialId: '', + }); + expect(agentIntegrationPersistenceService.removeCredentialIntegration).toHaveBeenCalledWith( + agent, + 'slack', + '', + { broadcast: false }, + ); + }); + it('starts Slack app setup with the temporary app configuration token', async () => { const slackAppSetupService = mock(); slackAppSetupService.createApp.mockResolvedValue({ diff --git a/packages/cli/src/modules/agents/__tests__/agent-validation.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-validation.service.test.ts index ed840009329..4decee02d9c 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-validation.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-validation.service.test.ts @@ -548,6 +548,57 @@ describe('AgentValidationService — structured issues', () => { ); }); + it('flags a vector store whose derived tool name collides with a configured tool', async () => { + const { service, agentRepository } = makeService(); + agentRepository.findByIdAndProjectId.mockResolvedValue( + makeAgent( + { + ...runnableConfig, + tools: [{ type: 'custom', id: 'search_product_docs' }], + vectorStores: [ + { + provider: 'qdrant', + name: 'product-docs', + credential: 'qdrant-cred', + useWhen: 'Search product docs', + embedding: { + model: 'openai/text-embedding-3-small', + credential: 'embed-cred', + }, + collectionName: 'product-docs', + }, + ], + }, + {}, + { + tools: { + search_product_docs: { + code: '', + descriptor: { name: 'search_product_docs' }, + }, + } as unknown as Agent['tools'], + }, + ), + ); + + const result = await service.validateAgentConfiguration( + agentId, + projectId, + makeCredentialProvider([{ id: 'openai-main', type: 'openAiApi' }]), + ); + + expect(result.status).toBe('invalid'); + expect(result.issues).toEqual( + expect.arrayContaining([ + { + code: 'invalid_value', + path: 'vectorStores.0.name', + capability: { kind: 'vectorStore', id: 'product-docs', index: 0 }, + }, + ]), + ); + }); + it('flags an enabled task with an invalid schedule while keeping its body available, but ignores the same invalid body on a disabled task', async () => { const { service, agentRepository, agentTaskRepository } = makeService(); agentTaskRepository.findByAgentId.mockResolvedValue([ diff --git a/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts b/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts index 75d76a37bbc..d85312560ad 100644 --- a/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts @@ -1007,10 +1007,28 @@ describe('AgentsBuilderToolsService', () => { expect(result).toEqual({ ok: false, - errors: expect.arrayContaining([ - expect.objectContaining({ path: 'model' }), - expect.objectContaining({ path: 'credential' }), - ]), + errors: expect.arrayContaining([expect.objectContaining({ path: 'model' })]), + }); + expect(agentsService.updateConfig).not.toHaveBeenCalled(); + }); + + it('write_config rejects a non-string model with a structured error instead of throwing', async () => { + const { service, agentsService } = makeService(); + const currentConfig = { ...baseConfig, integrations: [] }; + const malformedConfig = { ...currentConfig, model: 123 }; + agentsService.findById.mockResolvedValue(makeAgent(baseConfig)); + + const result = await getJsonTool(service, BUILDER_TOOLS.WRITE_CONFIG).handler!( + { + baseConfigHash: getAgentConfigHash(currentConfig), + json: JSON.stringify(malformedConfig), + }, + ctx, + ); + + expect(result).toEqual({ + ok: false, + errors: expect.arrayContaining([expect.objectContaining({ path: 'model' })]), }); expect(agentsService.updateConfig).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/modules/agents/agent-integration-persistence.service.ts b/packages/cli/src/modules/agents/agent-integration-persistence.service.ts index e4ef26ed619..eb844363718 100644 --- a/packages/cli/src/modules/agents/agent-integration-persistence.service.ts +++ b/packages/cli/src/modules/agents/agent-integration-persistence.service.ts @@ -1,5 +1,6 @@ import { AgentIntegrationSchema, + isDraftIntegration, type AgentIntegrationConfig, type ChatIntegrationDescriptor, } from '@n8n/api-types'; @@ -66,7 +67,7 @@ export class AgentIntegrationPersistenceService { const validated = parseResult.data; const { type, credentialId } = validated; - if (credentialId === '') { + if (isDraftIntegration(validated)) { throw new UserError('Credential integration requires a credential ID.'); } @@ -74,7 +75,7 @@ export class AgentIntegrationPersistenceService { // before setup completes) so connecting a real credential replaces it // instead of leaving both the draft and the connected entry behind. const existing = (agent.integrations ?? []).filter( - (i) => !(i.type === type && i.credentialId === ''), + (i) => !(i.type === type && isDraftIntegration(i)), ); const alreadyExists = existing.some((i) => i.type === type && i.credentialId === credentialId); diff --git a/packages/cli/src/modules/agents/agent-integrations.controller.ts b/packages/cli/src/modules/agents/agent-integrations.controller.ts index a3c9abc1de9..3e6eb241bc1 100644 --- a/packages/cli/src/modules/agents/agent-integrations.controller.ts +++ b/packages/cli/src/modules/agents/agent-integrations.controller.ts @@ -1,6 +1,7 @@ import { AgentDisconnectIntegrationDto, AgentIntegrationSchema, + isDraftIntegration, type AgentIntegrationStatusResponse, CreateSlackAgentAppDto, type CreateSlackAgentAppResponse, @@ -220,7 +221,7 @@ export class AgentIntegrationsController { // them as disconnected so channel-setup UIs don't render an already- // connected state and hide their own setup form. const chatIntegrations = (agent.integrations ?? []) - .filter((i) => i.credentialId !== '') + .filter((i) => !isDraftIntegration(i)) .map((i) => ({ type: i.type, credentialId: i.credentialId, diff --git a/packages/cli/src/modules/agents/agent-model-catalog.service.ts b/packages/cli/src/modules/agents/agent-model-catalog.service.ts index c0955e37617..b34a6eb7b82 100644 --- a/packages/cli/src/modules/agents/agent-model-catalog.service.ts +++ b/packages/cli/src/modules/agents/agent-model-catalog.service.ts @@ -6,7 +6,7 @@ import { Service } from '@n8n/di'; import { isModelDiscoveryProvider } from '@n8n/ai-utilities/model-discovery'; import { BuilderModelLiveLookupService } from './builder/builder-model-live-lookup.service'; -import { LLM_PROVIDER_DEFAULTS } from './builder/interactive/llm-provider-defaults'; +import { LLM_PROVIDER_DEFAULTS } from './llm-provider-defaults'; /** Google's models API returns ids as `models/`; the AI SDK expects the bare id. */ const GOOGLE_MODEL_ID_PREFIX = 'models/'; diff --git a/packages/cli/src/modules/agents/agent-publish.service.ts b/packages/cli/src/modules/agents/agent-publish.service.ts index e9e147253fe..07bf213f067 100644 --- a/packages/cli/src/modules/agents/agent-publish.service.ts +++ b/packages/cli/src/modules/agents/agent-publish.service.ts @@ -1,5 +1,6 @@ import { type AgentConfigValidationResponse, + isDraftIntegration, type AgentJsonConfig, type AgentSkill, type AgentVersionListItemDto, @@ -202,7 +203,7 @@ export class AgentPublishService { const baseIntegrations = agent.integrations ?? []; const integrations = ignoreDraftIntegrations - ? baseIntegrations.filter((integration) => integration.credentialId !== '') + ? baseIntegrations.filter((integration) => !isDraftIntegration(integration)) : baseIntegrations; const validation = targetHistory diff --git a/packages/cli/src/modules/agents/agent-validation.service.ts b/packages/cli/src/modules/agents/agent-validation.service.ts index c0a8f3d0e1b..f73c2ce7dac 100644 --- a/packages/cli/src/modules/agents/agent-validation.service.ts +++ b/packages/cli/src/modules/agents/agent-validation.service.ts @@ -4,6 +4,9 @@ import { getRequiredNodeCredentialSlots } from '@n8n/ai-utilities/node-catalog'; import { AgentModelSchema, agentTaskSchema, + findVectorStoreToolNameCollisions, + isDraftAgentConfig, + isDraftIntegration, type AgentConfigValidationIssue, type AgentConfigValidationIssueCode, type AgentConfigValidationResponse, @@ -20,7 +23,7 @@ import { isMcpOAuth2Authentication, NodeHelpers, type INodeParameters } from 'n8 import { getMissingSkillIds } from '@/modules/agents/utils/agent-missing-skill-ids'; import { NodeTypes } from '@/node-types'; -import { LLM_PROVIDER_DEFAULTS } from './builder/interactive/llm-provider-defaults'; +import { LLM_PROVIDER_DEFAULTS } from './llm-provider-defaults'; import type { AgentHistory } from './entities/agent-history.entity'; import type { Agent } from './entities/agent.entity'; import { ChatIntegrationRegistry } from './integrations/agent-chat-integration'; @@ -255,6 +258,7 @@ export class AgentValidationService { const { agentsById, workflowsByName } = await this.prefetchReferenceLookups(ctx); this.collectCoreIssues(config, issues); + this.collectVectorStoreIssues(config, issues); await this.collectMainCredentialIssues(config, findCredential, issues); this.collectSubAgentRefIssues(ctx, agentsById, issues); this.collectSkillIssues(config, ctx.skills, issues); @@ -315,7 +319,7 @@ export class AgentValidationService { issues.push(agentIssue('missing_required', 'instructions')); } - if (!config.model?.trim()) { + if (isDraftAgentConfig(config)) { issues.push(agentIssue('missing_required', 'model')); } else if (!AgentModelSchema.safeParse(config.model).success) { issues.push(agentIssue('invalid_value', 'model')); @@ -413,6 +417,29 @@ export class AgentValidationService { } } + /** + * A vector store registers a `search_` tool at runtime; a + * collision with a configured tool name only fails once the agent is built. + * The write gate (AgentConfigService.validateConfig) checks this too — this + * re-check covers configs that reached the entity through other paths + * (e.g. history restore). + */ + private collectVectorStoreIssues(config: AgentJsonConfig, issues: AgentConfigValidationIssue[]) { + const collisions = new Set(findVectorStoreToolNameCollisions(config)); + const stores = config.vectorStores ?? []; + for (let index = 0; index < stores.length; index++) { + const store = stores[index]; + if (!collisions.has(`search_${store.name.replace(/-/g, '_')}`)) continue; + issues.push( + issue('invalid_value', `vectorStores.${index}.name`, { + kind: 'vectorStore', + id: store.name, + index, + }), + ); + } + } + private async collectChannelIssues( integrations: AgentIntegrationConfig[], findCredential: FindCredential, @@ -426,12 +453,11 @@ export class AgentValidationService { id: integration.type, index, }; - const credentialId = integration.credentialId?.trim(); - - if (!credentialId) { + if (isDraftIntegration(integration)) { issues.push(issue('missing_credential', path, capability)); continue; } + const credentialId = integration.credentialId.trim(); const credential = await this.findCredentialSafe(findCredential, credentialId); if (!credential) { diff --git a/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts b/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts index fa814f6e0c2..e87b1738bae 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts @@ -15,7 +15,8 @@ import { PROVIDER_CAPABILITIES, resolvePromptCaching, AgentJsonConfigSchema, - RunnableAgentJsonConfigSchema, + isDraftAgentConfig, + isDraftIntegration, sanitizeAgentJsonConfig, tryParseConfigJson, type AgentJsonConfig, @@ -137,14 +138,28 @@ function snapshotFromConfig(config: AgentJsonConfig | null): AgentConfigSnapshot } /** - * Draft writes (empty `model`, no `credential`) are only for agents that - * don't have a model yet. Once the stored config has a model, require the - * runnable schema so a builder write can't wipe it back into an unrunnable - * draft. + * Once the stored config has a model, a builder write can't clear it back to + * a draft (`model: ""`). A missing credential does NOT reject the write — + * it surfaces as a `missing_credential` validation issue instead. */ function parseBuilderWriteConfig(incoming: unknown, currentConfig: AgentJsonConfig | null) { - const schema = currentConfig?.model ? RunnableAgentJsonConfigSchema : AgentJsonConfigSchema; - return schema.safeParse(sanitizeAgentJsonConfig(incoming)); + const sanitized = sanitizeAgentJsonConfig(incoming); + if ( + !isDraftAgentConfig(currentConfig) && + isDraftAgentConfig(sanitized as { model?: string } | null | undefined) + ) { + return { + success: false as const, + error: new z.ZodError([ + { + code: z.ZodIssueCode.custom, + path: ['model'], + message: 'Model cannot be cleared once set', + }, + ]), + }; + } + return AgentJsonConfigSchema.safeParse(sanitized); } /** @@ -626,8 +641,8 @@ export class AgentsBuilderToolsService { listIntegrationCredentialIds: async () => { const agent = await this.agentsService.findById(agentId, projectId); return (agent?.integrations ?? []) - .map((integration) => integration.credentialId) - .filter((credentialId) => credentialId.length > 0); + .filter((integration) => !isDraftIntegration(integration)) + .map((integration) => integration.credentialId); }, }), buildAskEmbeddingCredentialTool({ @@ -657,8 +672,8 @@ export class AgentsBuilderToolsService { listIntegrationCredentialIds: async () => { const agent = await this.agentsService.findById(agentId, projectId); return (agent?.integrations ?? []) - .map((integration) => integration.credentialId) - .filter((credentialId) => credentialId.length > 0); + .filter((integration) => !isDraftIntegration(integration)) + .map((integration) => integration.credentialId); }, listChatIntegrationTypes: () => this.agentIntegrationPersistenceService diff --git a/packages/cli/src/modules/agents/builder/interactive/__tests__/resolve-llm.tool.test.ts b/packages/cli/src/modules/agents/builder/interactive/__tests__/resolve-llm.tool.test.ts index 27feb815ba5..772880be050 100644 --- a/packages/cli/src/modules/agents/builder/interactive/__tests__/resolve-llm.tool.test.ts +++ b/packages/cli/src/modules/agents/builder/interactive/__tests__/resolve-llm.tool.test.ts @@ -1,7 +1,7 @@ import type { CredentialListItem, CredentialProvider } from '@n8n/agents'; import type { Mock } from 'vitest'; -import { LLM_PROVIDER_DEFAULTS, LLM_PROVIDER_PRIORITY } from '../llm-provider-defaults'; +import { LLM_PROVIDER_DEFAULTS, LLM_PROVIDER_PRIORITY } from '../../../llm-provider-defaults'; import type { FreeCreditsProvisioner, ModelLookup } from '../resolve-llm.tool'; import { buildResolveLlmTool } from '../resolve-llm.tool'; diff --git a/packages/cli/src/modules/agents/builder/interactive/resolve-llm.tool.ts b/packages/cli/src/modules/agents/builder/interactive/resolve-llm.tool.ts index 2d088cf8c79..8f4c431d973 100644 --- a/packages/cli/src/modules/agents/builder/interactive/resolve-llm.tool.ts +++ b/packages/cli/src/modules/agents/builder/interactive/resolve-llm.tool.ts @@ -8,7 +8,7 @@ import { LLM_PROVIDER_DEFAULTS, LLM_PROVIDER_PRIORITY, type LlmProviderDefault, -} from './llm-provider-defaults'; +} from '../../llm-provider-defaults'; export interface ModelLookup { list( diff --git a/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts index 18ba71f0f82..0989ee9c1d9 100644 --- a/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts @@ -1,4 +1,4 @@ -import { AgentJsonConfigSchema, AgentModelSchema } from '@n8n/api-types'; +import { AgentJsonConfigBaseSchema, AgentModelSchema } from '@n8n/api-types'; import type { JSONSchema7 } from 'json-schema'; import type { ZodObject, ZodRawShape } from 'zod'; import { z } from 'zod'; @@ -41,7 +41,7 @@ const BuilderPromptMemoryConfigSchema = z.object({ .optional(), }); -const BuilderPromptAgentJsonConfigSchema = AgentJsonConfigSchema.extend({ +const BuilderPromptAgentJsonConfigSchema = AgentJsonConfigBaseSchema.extend({ memory: BuilderPromptMemoryConfigSchema.optional(), }); diff --git a/packages/cli/src/modules/agents/json-config/__tests__/sanitize-unknown-agent-credentials.test.ts b/packages/cli/src/modules/agents/json-config/__tests__/sanitize-unknown-agent-credentials.test.ts index 9771d66e04f..06f5201a603 100644 --- a/packages/cli/src/modules/agents/json-config/__tests__/sanitize-unknown-agent-credentials.test.ts +++ b/packages/cli/src/modules/agents/json-config/__tests__/sanitize-unknown-agent-credentials.test.ts @@ -22,11 +22,15 @@ describe('sanitizeUnknownAgentCredentials', () => { it('preserves known credential fields', () => { const result = sanitizeUnknownAgentCredentials( - { credential: 'known-cred', name: 'Agent' }, + { credential: 'known-cred', model: 'anthropic/claude-sonnet-4-5', name: 'Agent' }, accessibleCredentialIds, ); - expect(result).toEqual({ credential: 'known-cred', name: 'Agent' }); + expect(result).toEqual({ + credential: 'known-cred', + model: 'anthropic/claude-sonnet-4-5', + name: 'Agent', + }); }); it('preserves managed proxy credential tokens only for episodic memory embeddings', () => { @@ -269,4 +273,25 @@ describe('sanitizeUnknownAgentCredentials', () => { 'credential', ); }); + + it('clears a top-level credential when model is empty', () => { + const result = sanitizeUnknownAgentCredentials( + { model: '', credential: 'known-cred' }, + accessibleCredentialIds, + ); + + expect(result).toEqual({ model: '', credential: '' }); + }); + + it('preserves a top-level credential when model is set', () => { + const result = sanitizeUnknownAgentCredentials( + { model: 'anthropic/claude-sonnet-4-5', credential: 'known-cred' }, + accessibleCredentialIds, + ); + + expect(result).toEqual({ + model: 'anthropic/claude-sonnet-4-5', + credential: 'known-cred', + }); + }); }); diff --git a/packages/cli/src/modules/agents/json-config/sanitize-unknown-agent-credentials.ts b/packages/cli/src/modules/agents/json-config/sanitize-unknown-agent-credentials.ts index d83384ab543..947157d48c1 100644 --- a/packages/cli/src/modules/agents/json-config/sanitize-unknown-agent-credentials.ts +++ b/packages/cli/src/modules/agents/json-config/sanitize-unknown-agent-credentials.ts @@ -1,4 +1,4 @@ -import { MANAGED_CREDENTIAL_TOKEN } from '@n8n/api-types'; +import { isDraftAgentConfig, MANAGED_CREDENTIAL_TOKEN } from '@n8n/api-types'; function clearUnknownCredentialId( credentialId: unknown, @@ -99,6 +99,10 @@ function sanitizeUnknownCredentialsInValue( * Replace credential IDs that are not accessible to the agent project with `""`. * Walks the config recursively and only targets credential-like fields: * `credential`, `credentialId`, and `credentials.*.id`. + * + * A top-level credential without a model is also cleared so legacy rows + * converge to a clean draft instead of failing the `credential ⇒ model` + * schema refine. */ export function sanitizeUnknownAgentCredentials( raw: unknown, @@ -108,5 +112,23 @@ export function sanitizeUnknownAgentCredentials( return raw; } - return sanitizeUnknownCredentialsInValue(raw, accessibleCredentialIds); + const sanitized = sanitizeUnknownCredentialsInValue(raw, accessibleCredentialIds); + if (!isRecord(sanitized)) { + return sanitized; + } + + const model = typeof sanitized.model === 'string' ? sanitized.model : undefined; + if ( + isDraftAgentConfig({ model }) && + typeof sanitized.credential === 'string' && + sanitized.credential !== '' + ) { + sanitized.credential = ''; + } + + return sanitized; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); } diff --git a/packages/cli/src/modules/agents/builder/interactive/llm-provider-defaults.ts b/packages/cli/src/modules/agents/llm-provider-defaults.ts similarity index 100% rename from packages/cli/src/modules/agents/builder/interactive/llm-provider-defaults.ts rename to packages/cli/src/modules/agents/llm-provider-defaults.ts diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelModal.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelModal.test.ts index e7a10b3b93e..f2ff4e14f1b 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelModal.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelModal.test.ts @@ -1,9 +1,13 @@ -import { mount } from '@vue/test-utils'; +import { mount, VueWrapper } from '@vue/test-utils'; import { ref } from 'vue'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi, beforeEach } from 'vitest'; import AgentChannelModal from '../components/AgentChannelModal.vue'; +type SlackSetupStubWrapper = VueWrapper<{ + disconnectSlackApp: () => Promise; +}>; + vi.mock('@n8n/i18n', () => ({ useI18n: () => ({ baseText: (key: string) => key, @@ -15,6 +19,8 @@ const catalog = ref([ { type: 'linear', label: 'Linear', icon: 'zap' }, ]); +const disconnectMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); + vi.mock('../composables/useAgentIntegrationsCatalog', () => ({ useAgentIntegrationsCatalog: () => ({ catalog, @@ -32,7 +38,7 @@ vi.mock('../composables/useAgentIntegrationStatus', () => ({ errorIsConflict: ref({}), isConnected: () => false, connect: vi.fn(), - disconnect: vi.fn(), + disconnect: disconnectMock, }), })); @@ -79,7 +85,7 @@ function mountModal(props: Record) { N8nText: { template: '' }, AgentChannelListItem: { template: '
  • ' }, AgentChannelSlackSetup: { - props: ['mode'], + props: ['mode', 'disconnectSlackApp'], template: '
    ', }, AgentChannelLinearSetup: { @@ -96,6 +102,10 @@ function mountModal(props: Record) { } describe('AgentChannelModal', () => { + beforeEach(() => { + disconnectMock.mockClear(); + }); + it('renders the channel list for the list view', () => { const wrapper = mountModal({ view: 'list' }); @@ -115,4 +125,20 @@ describe('AgentChannelModal', () => { const linearSetup = wrapper.find('[data-testid="linear-setup"]'); expect(linearSetup.attributes('data-mode')).toBe('edit'); }); + + it('disconnects a draft slack channel and closes the modal', async () => { + const wrapper = mountModal({ + view: 'slack_edit', + connectedChannels: ['slack'], + }); + + const slackSetup = wrapper.findComponent( + '[data-testid="slack-setup"]', + ) as SlackSetupStubWrapper; + await slackSetup.vm.disconnectSlackApp(); + + expect(disconnectMock).toHaveBeenCalledWith('slack', ''); + expect(wrapper.emitted('channel-disconnected')).toEqual([['slack']]); + expect(wrapper.emitted('update:open')).toEqual([[false]]); + }); }); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentChannelModal.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentChannelModal.vue index 2c4496433f7..6a3143238cc 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentChannelModal.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentChannelModal.vue @@ -207,9 +207,9 @@ async function setupSlackApp(appConfigurationToken: string): Promise { } async function handleDisconnected(channelType: string) { - const credentialId = connectedCredentials.value[channelType]; - if (!credentialId) return; - + // Draft channels (configured but missing a credential) have no connected + // credential — send '' so the backend removes the draft entry by type. + const credentialId = connectedCredentials.value[channelType] ?? ''; await disconnect(channelType, credentialId); emit('channel-disconnected', channelType); emit('agent-changed');