From e5b1ac9f38fcd4f2acc84838d2d951ac07abb5ad Mon Sep 17 00:00:00 2001 From: Robin Braumann <50590409+bjorger@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:55:51 +0000 Subject: [PATCH] feat(core): Build agent drafts before asking for setup in the agent builder (no-changelog) (#34615) Co-authored-by: Cursor --- packages/@n8n/agents/src/index.ts | 2 + .../__tests__/planner-todos-tool.test.ts | 52 ++ .../src/runtime/tools/planner-todos-tool.ts | 66 +++ .../agents/src/runtime/tools/todos-core.ts | 51 ++ .../src/runtime/tools/write-todos-tool.ts | 58 +-- .../instance-ai/src/agent/system-prompt.ts | 2 +- .../src/runtime/run-state-registry.ts | 6 +- .../__tests__/build-agent.tool.test.ts | 86 ++++ .../tools/orchestration/build-agent.tool.ts | 87 +++- .../@n8n/instance-ai/src/tools/tool-ids.ts | 6 + .../__tests__/ai.controller.test.ts | 5 +- packages/cli/src/controllers/ai.controller.ts | 38 +- ...nt-integration-persistence.service.test.ts | 17 + .../agent-integrations.controller.test.ts | 23 +- .../__tests__/agent-publish.service.test.ts | 24 + .../agents-builder-tools.service.test.ts | 200 +++++++- .../agent-integration-persistence.service.ts | 7 +- .../agents/agent-integrations.controller.ts | 18 +- .../modules/agents/agent-publish.service.ts | 45 +- .../agents/agent-validation.service.ts | 9 +- ...ents-builder-model-recommendations.test.ts | 41 +- .../__tests__/agents-builder-prompts.test.ts | 320 ------------ .../agents-builder.service.session.test.ts | 10 +- .../__tests__/verify-mcp-server.tool.test.ts | 135 ++++- .../agents-builder-model-recommendations.ts | 10 +- .../agents/builder/agents-builder-prompts.ts | 130 +++-- .../builder/agents-builder-tools.service.ts | 198 ++++++-- .../agents/builder/agents-builder.service.ts | 13 +- .../agents/builder/builder-tool-names.ts | 1 + .../__tests__/finish-setup.tool.test.ts | 353 +++++++++++++ .../__tests__/resolve-llm.tool.test.ts | 304 ++++++++++- .../interactive/ask-credential.tool.ts | 7 +- .../interactive/configure-channel.tool.ts | 2 +- .../builder/interactive/finish-setup.tool.ts | 470 ++++++++++++++++++ .../agents/builder/interactive/index.ts | 1 + .../interactive/llm-provider-defaults.ts | 15 + .../builder/interactive/resolve-llm.tool.ts | 238 ++++++--- .../builder/prompts/config-mutation.prompt.ts | 11 +- .../builder/prompts/config-rules.prompt.ts | 13 +- .../builder/prompts/initial-build.prompt.ts | 67 +++ .../builder/prompts/llm-selection.prompt.ts | 9 +- .../agents/builder/prompts/memory.prompt.ts | 4 +- .../builder/prompts/planner-todos.prompt.ts | 26 + .../agents/builder/prompts/tools.prompt.ts | 24 +- .../builder/resolve-integration.tool.ts | 2 +- .../builder/skills/__tests__/index.test.ts | 4 +- .../builder/skills/external-services.skill.ts | 353 +++++++++++++ .../modules/agents/builder/skills/index.ts | 8 +- .../builder/skills/integrations.skill.ts | 127 ----- .../agents/builder/skills/mcp.skill.ts | 176 ------- .../agents/builder/skills/memory.skill.ts | 4 +- .../agents/builder/skills/node-tools.skill.ts | 95 ---- .../builder/skills/resource-locators.skill.ts | 21 +- .../builder/skills/target-skills.skill.ts | 21 +- .../builder/skills/target-tasks.skill.ts | 22 +- .../agents/builder/verify-mcp-server.tool.ts | 43 +- .../__tests__/slack-app-setup.service.test.ts | 4 +- .../integrations/slack-app-setup.service.ts | 1 + .../__tests__/instance-ai.service.test.ts | 40 ++ .../instance-ai/instance-ai.service.ts | 28 +- .../__tests__/free-ai-credits.service.test.ts | 102 ++++ .../src/services/free-ai-credits.service.ts | 58 +++ .../frontend/@n8n/i18n/src/locales/en.json | 1 + .../agents/__tests__/AgentBuilderView.test.ts | 149 +++--- .../AgentCapabilitiesSection.test.ts | 94 +++- .../__tests__/useAgentChannelSetup.test.ts | 55 ++ .../__tests__/useAgentConfigAutosave.test.ts | 23 +- .../agents/components/AgentBuilderHeader.vue | 4 +- .../components/AgentCapabilitiesSection.vue | 23 +- .../agents/components/AgentMemoryPanel.vue | 6 +- .../agents/components/AgentSubAgentsPanel.vue | 7 +- .../composables/useAgentChannelSetup.ts | 37 +- .../composables/useAgentConfigAutosave.ts | 13 +- .../agents/views/AgentBuilderView.vue | 89 ++-- .../ai/instanceAi/InstanceAiThreadView.vue | 1 - .../InstanceAiCredentialSetup.test.ts | 64 +++ .../__tests__/InstanceAiThreadView.test.ts | 7 +- .../__tests__/canvasPreview.utils.test.ts | 62 +++ .../__tests__/useCanvasPreview.test.ts | 183 ++----- .../ai/instanceAi/canvasPreview.utils.ts | 110 ++-- .../components/InstanceAiAgentPreview.vue | 50 +- .../components/InstanceAiCredentialSetup.vue | 99 +++- .../ai/instanceAi/useCanvasPreview.ts | 47 +- 83 files changed, 3912 insertions(+), 1525 deletions(-) create mode 100644 packages/@n8n/agents/src/runtime/__tests__/planner-todos-tool.test.ts create mode 100644 packages/@n8n/agents/src/runtime/tools/planner-todos-tool.ts create mode 100644 packages/@n8n/agents/src/runtime/tools/todos-core.ts delete mode 100644 packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts create mode 100644 packages/cli/src/modules/agents/builder/interactive/__tests__/finish-setup.tool.test.ts create mode 100644 packages/cli/src/modules/agents/builder/interactive/finish-setup.tool.ts create mode 100644 packages/cli/src/modules/agents/builder/prompts/initial-build.prompt.ts create mode 100644 packages/cli/src/modules/agents/builder/prompts/planner-todos.prompt.ts create mode 100644 packages/cli/src/modules/agents/builder/skills/external-services.skill.ts delete mode 100644 packages/cli/src/modules/agents/builder/skills/integrations.skill.ts delete mode 100644 packages/cli/src/modules/agents/builder/skills/mcp.skill.ts delete mode 100644 packages/cli/src/modules/agents/builder/skills/node-tools.skill.ts create mode 100644 packages/cli/src/services/__tests__/free-ai-credits.service.test.ts create mode 100644 packages/cli/src/services/free-ai-credits.service.ts diff --git a/packages/@n8n/agents/src/index.ts b/packages/@n8n/agents/src/index.ts index ac178263d84..0eb10673f4b 100644 --- a/packages/@n8n/agents/src/index.ts +++ b/packages/@n8n/agents/src/index.ts @@ -281,6 +281,8 @@ export type { SubAgentTaskDifficulty, } from './runtime/tools/delegate-sub-agent-tool'; export { WRITE_TODOS_TOOL_NAME, createWriteTodosTool } from './runtime/tools/write-todos-tool'; +export { createPlannerTodosTool } from './runtime/tools/planner-todos-tool'; +export type { CreatePlannerTodosToolOptions } from './runtime/tools/planner-todos-tool'; export type { CreateWriteTodosToolOptions } from './runtime/tools/write-todos-tool'; export { createEmbeddingModel } from './runtime/model/model-factory'; export { generateTitleFromMessage } from './runtime/memory/title-generation'; diff --git a/packages/@n8n/agents/src/runtime/__tests__/planner-todos-tool.test.ts b/packages/@n8n/agents/src/runtime/__tests__/planner-todos-tool.test.ts new file mode 100644 index 00000000000..6cfde8ddd34 --- /dev/null +++ b/packages/@n8n/agents/src/runtime/__tests__/planner-todos-tool.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest'; + +import { Agent } from '../../sdk/agent'; +import { isZodSchema } from '../../utils/zod'; +import { createPlannerTodosTool } from '../tools/planner-todos-tool'; +import { isSdkOwnedBuiltInTool } from '../tools/sdk-owned-tool'; + +const samplePlannerTodos = [ + { id: 'a', content: 'Resolve model', status: 'blocked' as const }, + { id: 'b', content: 'Write instructions', status: 'in_progress' as const }, +]; + +describe('createPlannerTodosTool', () => { + it('accepts bare items including blocked status and echoes them with a count', async () => { + const tool = createPlannerTodosTool(); + + await expect( + tool.handler?.( + { todos: samplePlannerTodos }, + { runId: 'r1', persistence: { threadId: 't1', resourceId: 'u1' } }, + ), + ).resolves.toEqual({ + status: 'ok', + todoCount: 2, + todos: samplePlannerTodos, + }); + }); + + it('rejects items carrying delegation fields', () => { + const tool = createPlannerTodosTool(); + expect(isZodSchema(tool.inputSchema)).toBe(true); + if (!isZodSchema(tool.inputSchema)) { + throw new Error('Expected Zod input schema'); + } + + expect( + tool.inputSchema.safeParse({ + todos: [{ id: 'a', content: 'x', status: 'pending', difficulty: 'low' }], + }).success, + ).toBe(false); + }); + + it('registers on an agent as an official SDK built-in', () => { + expect(() => + new Agent('parent') + .model('openai', 'gpt-4o-mini') + .instructions('t') + .tool(createPlannerTodosTool()), + ).not.toThrow(); + expect(isSdkOwnedBuiltInTool(createPlannerTodosTool())).toBe(true); + }); +}); diff --git a/packages/@n8n/agents/src/runtime/tools/planner-todos-tool.ts b/packages/@n8n/agents/src/runtime/tools/planner-todos-tool.ts new file mode 100644 index 00000000000..2c37aaa428d --- /dev/null +++ b/packages/@n8n/agents/src/runtime/tools/planner-todos-tool.ts @@ -0,0 +1,66 @@ +import { z } from 'zod'; + +import { withSdkOwnedBuiltInMetadata } from './sdk-owned-tool'; +import { + buildTodosInputSchema, + buildTodosOutputSchema, + todosEchoHandler, + todoStatusSchema, +} from './todos-core'; +import { WRITE_TODOS_TOOL_NAME } from './write-todos-tool'; +import { Tool } from '../../sdk/tool'; +import type { BuiltTool } from '../../types/sdk/tool'; + +const plannerTodoItemSchema = z + .object({ + id: z.string().min(1).describe('Stable identifier for this task within the current plan.'), + content: z + .string() + .min(1) + .describe( + 'Concrete, self-contained task description. For blocked tasks, state exactly what user input is missing.', + ), + status: todoStatusSchema, + }) + .strict(); + +const PLANNER_TODOS_DESCRIPTION = + 'Create or update a structured task list for the current task. Use it to decompose the work, ' + + 'track progress, and record which tasks are blocked on user input. Use it for every multi-step ' + + 'request. This tool only updates the task list; it does not perform work or ask the user.'; + +const PLANNER_TODOS_SYSTEM_INSTRUCTION = [ + 'write_todos maintains your plan for the current task. It never asks the user or performs work itself.', + 'WHEN TO USE write_todos:', + '- Every multi-step request, before other tool calls.', + '- You need to track progress or record tasks blocked on user input.', + 'WHEN NOT TO USE write_todos:', + '- Purely conversational replies with no task work.', + 'HOW TO USE write_todos:', + '- Write concrete, self-contained tasks; mark the first active task in_progress immediately.', + '- Mark a task blocked when it cannot proceed without user input, and state exactly what input is missing in the task content.', + '- Marking a task blocked IS the action for now. Do not ask the user about it while any non-blocked task remains; continue with unblocked tasks.', + '- When only blocked tasks remain, end your turn with a summary of what is missing; unblock and finish them in later turns as the user provides input.', + '- Update task status as soon as work completes; do not batch completions at the end.', + '- Do not call write_todos multiple times in parallel; send one full list update at a time.', +].join('\n'); + +export interface CreatePlannerTodosToolOptions { + /** Override the model-visible tool description. */ + description?: string; + /** Override the model-visible system instruction. */ + systemInstruction?: string; +} + +/** Planner-only variant of `write_todos` — no delegation fields or guidance. */ +export function createPlannerTodosTool(options: CreatePlannerTodosToolOptions = {}): BuiltTool { + const tool = new Tool(WRITE_TODOS_TOOL_NAME) + .description(options.description ?? PLANNER_TODOS_DESCRIPTION) + .systemInstruction(options.systemInstruction ?? PLANNER_TODOS_SYSTEM_INSTRUCTION) + .input(buildTodosInputSchema(plannerTodoItemSchema)) + .output(buildTodosOutputSchema(plannerTodoItemSchema)) + .handler(todosEchoHandler) + .build(); + + return withSdkOwnedBuiltInMetadata(tool); +} diff --git a/packages/@n8n/agents/src/runtime/tools/todos-core.ts b/packages/@n8n/agents/src/runtime/tools/todos-core.ts new file mode 100644 index 00000000000..7ad23ed029c --- /dev/null +++ b/packages/@n8n/agents/src/runtime/tools/todos-core.ts @@ -0,0 +1,51 @@ +import { z } from 'zod'; + +export const todoStatusSchema = z.enum([ + 'pending', + 'in_progress', + 'completed', + 'blocked', + 'cancelled', +]); + +/** Wraps a todo item schema in the shared `{ todos: [...] }` input shape with duplicate-id validation. */ +export function buildTodosInputSchema>(todoItemSchema: T) { + return z + .object({ + todos: z + .array(todoItemSchema) + .describe('Full task list for the current run. Replaces any previous list.'), + }) + .superRefine((value, ctx) => { + const seen = new Set(); + for (const [index, todo] of value.todos.entries()) { + const id = todo.id; + if (seen.has(id)) { + ctx.addIssue({ + code: 'custom', + message: `Duplicate todo id "${id}". Each task must have a unique id.`, + path: ['todos', index, 'id'], + }); + } + seen.add(id); + } + }); +} + +export function buildTodosOutputSchema(todoItemSchema: T) { + return z.object({ + status: z.literal('ok'), + todoCount: z.number(), + todos: z.array(todoItemSchema), + }); +} + +/** Echo handler shared by all todo tools — state lives in the transcript, not the tool. */ +export async function todosEchoHandler(input: { todos: T[] }) { + const todos = [...input.todos]; + return await Promise.resolve({ + status: 'ok' as const, + todoCount: todos.length, + todos, + }); +} diff --git a/packages/@n8n/agents/src/runtime/tools/write-todos-tool.ts b/packages/@n8n/agents/src/runtime/tools/write-todos-tool.ts index 3e15f6db87e..c8b4d227f3c 100644 --- a/packages/@n8n/agents/src/runtime/tools/write-todos-tool.ts +++ b/packages/@n8n/agents/src/runtime/tools/write-todos-tool.ts @@ -5,13 +5,17 @@ import { SUB_AGENT_TASK_DIFFICULTIES, } from './delegate-sub-agent-tool'; import { withSdkOwnedBuiltInMetadata } from './sdk-owned-tool'; +import { + buildTodosInputSchema, + buildTodosOutputSchema, + todosEchoHandler, + todoStatusSchema, +} from './todos-core'; import { Tool } from '../../sdk/tool'; import type { BuiltTool } from '../../types/sdk/tool'; export const WRITE_TODOS_TOOL_NAME = 'write_todos'; -const todoStatusSchema = z.enum(['pending', 'in_progress', 'completed', 'blocked', 'cancelled']); - const todoDifficultySchema = z.enum(SUB_AGENT_TASK_DIFFICULTIES); function buildTodoItemSchema(delegateToolName: string) { @@ -41,38 +45,6 @@ function buildTodoItemSchema(delegateToolName: string) { }); } -type TodoItemSchema = ReturnType; - -function buildWriteTodosInputSchema(todoItemSchema: TodoItemSchema) { - return z - .object({ - todos: z - .array(todoItemSchema) - .describe('Full task list for the current run. Replaces any previous list.'), - }) - .superRefine((value, ctx) => { - const seen = new Set(); - for (const [index, todo] of value.todos.entries()) { - if (seen.has(todo.id)) { - ctx.addIssue({ - code: 'custom', - message: `Duplicate todo id "${todo.id}". Each task must have a unique id.`, - path: ['todos', index, 'id'], - }); - } - seen.add(todo.id); - } - }); -} - -function buildWriteTodosOutputSchema(todoItemSchema: TodoItemSchema) { - return z.object({ - status: z.literal('ok'), - todoCount: z.number(), - todos: z.array(todoItemSchema), - }); -} - function buildWriteTodosDescription(delegateToolName: string): string { return `Create or update a structured task list for complex agent work. Use it to decompose a larger request into concrete workstreams, track progress, and identify which tasks should be handled separately with ${delegateToolName}. Do not use it for trivial work, single-step tasks, or purely conversational answers. This tool only updates the task list; it does not run sub-agents or answer the user.`; } @@ -112,8 +84,8 @@ export interface CreateWriteTodosToolOptions { } /** - * Build the planner-only `write_todos` tool — lets a parent agent maintain a - * structured task list for complex work without auto-dispatching sub-agents. + * Build the delegation-aware `write_todos` tool — lets a parent agent maintain + * a structured task list and mark tasks for sub-agent delegation. */ export function createWriteTodosTool(options: CreateWriteTodosToolOptions = {}): BuiltTool { const delegateToolName = options.delegateToolName ?? DELEGATE_SUB_AGENT_TOOL_NAME; @@ -122,17 +94,9 @@ export function createWriteTodosTool(options: CreateWriteTodosToolOptions = {}): const tool = new Tool(WRITE_TODOS_TOOL_NAME) .description(buildWriteTodosDescription(delegateToolName)) .systemInstruction(buildWriteTodosSystemInstruction(delegateToolName)) - .input(buildWriteTodosInputSchema(todoItemSchema)) - .output(buildWriteTodosOutputSchema(todoItemSchema)) - .handler(async (input) => { - const todos = [...input.todos]; - - return await Promise.resolve({ - status: 'ok' as const, - todoCount: todos.length, - todos, - }); - }) + .input(buildTodosInputSchema(todoItemSchema)) + .output(buildTodosOutputSchema(todoItemSchema)) + .handler(todosEchoHandler) .build(); return withSdkOwnedBuiltInMetadata(tool); diff --git a/packages/@n8n/instance-ai/src/agent/system-prompt.ts b/packages/@n8n/instance-ai/src/agent/system-prompt.ts index 4d60f752ec7..9e53c9c1bdb 100644 --- a/packages/@n8n/instance-ai/src/agent/system-prompt.ts +++ b/packages/@n8n/instance-ai/src/agent/system-prompt.ts @@ -45,7 +45,7 @@ const INTENT_HINT = isAgentFeatureEnabled() : ''; const AGENT_BUILD_ROUTE = isAgentFeatureEnabled() - ? '\n- **Agent build or edit** (agent-anchored per the intent gate: chat or session interaction, cross-session memory, proactive or long-running operation, learning from feedback) → call `build-agent` right away and let the builder gather requirements — do not run your own requirement-gathering round first. The builder cannot see this conversation: its only knowledge is what you pass in `message`, so include ALL requirements, constraints, and answers already gathered in the first `message` (plus `name` for a new agent or `agentId` for an existing one, and `workflowContext` for workflows built this session). Each distinct agent the user asks for is its own build target — pass `name` or `agentId` again to create or switch agents (prefer the `agentId` returned by earlier build-agent results when switching back; a name used earlier in this conversation also switches back to that agent rather than duplicating it); calls without either continue the most recent target. When the user message includes an agent-preview reference / ``, call `get-session` to read the transcript first. For review/analysis/assessment requests (e.g. "review the tone", "how did it do", "assess its behavior"), answer directly from the transcript — do NOT call `build-agent` or otherwise modify the agent. Only call `build-agent` when the user explicitly asks to update, improve, or fix the agent; then pass the given `agentId` and put the concrete behavioral findings (failures, bad tool use, wrong answers) into `message` — do not ask the user to re-describe what already appears in the transcript. While a build is in progress, forward each user follow-up to `build-agent` near-verbatim and relay its `builderReply` back. When the builder needs user input it asks directly through interactive cards in this chat — do not re-ask those questions yourself; the tool call resumes with the user’s answer and returns the builder’s reply. When the user asks to publish, unpublish, activate, or make an agent live/usable, forward that intent to `build-agent` (the builder calls `publish_agent` / `unpublish_agent`) — never tell the user to open the agent editor and click Publish. Pass requests to add tools or capabilities to an agent to `build-agent` near-verbatim so the delegated builder can choose MCP, node-backed, provider, or custom tools. Do not build a workflow merely because the capability is reusable by the agent: multiple independent tools do not become one workflow. Use `workflow-builder` before `build-agent` only when one agent tool call must execute an ordered multi-node procedure, or when the user explicitly asks for a workflow that is reusable or callable outside the agent; then attach it via `workflowContext`. For questions about existing agents ("what agents do I have?") call `agents(action="list")` directly — no intent gate; and to edit an agent NOT built in this conversation, find its id via `agents(action="list")` and pass it as `agentId`.' + ? '\n- **Agent build or edit** (agent-anchored per the intent gate: chat or session interaction, cross-session memory, proactive or long-running operation, learning from feedback) → call `build-agent` right away and let the builder gather requirements — do not run your own requirement-gathering round first. The builder cannot see this conversation: its only knowledge is what you pass in `message`, so include ALL requirements, constraints, and answers already gathered in the first `message` (plus `name` + `createNew: true` for a new agent or `agentId` for an existing one, and `workflowContext` for workflows built this session). Each distinct agent the user asks for is its own build target — pass `name` or `agentId` again to create or switch agents (prefer the `agentId` returned by earlier build-agent results when switching back; a name used earlier in this conversation also switches back to that agent rather than duplicating it — unless `createNew: true` is passed); calls without either continue the most recent target. When the user message includes an agent-preview reference / ``, call `get-session` to read the transcript first. For review/analysis/assessment requests (e.g. "review the tone", "how did it do", "assess its behavior"), answer directly from the transcript — do NOT call `build-agent` or otherwise modify the agent. Only call `build-agent` when the user explicitly asks to update, improve, or fix the agent; then pass the given `agentId` and put the concrete behavioral findings (failures, bad tool use, wrong answers) into `message` — do not ask the user to re-describe what already appears in the transcript. While a build is in progress, forward each user follow-up to `build-agent` near-verbatim and relay its `builderReply` back. When the builder needs user input it asks directly through interactive cards in this chat — do not re-ask those questions yourself; the tool call resumes with the user’s answer and returns the builder’s reply. When the user asks to publish, unpublish, activate, or make an agent live/usable, forward that intent to `build-agent` (the builder calls `publish_agent` / `unpublish_agent`) — never tell the user to open the agent editor and click Publish. Pass requests to add tools or capabilities to an agent to `build-agent` near-verbatim so the delegated builder can choose MCP, node-backed, provider, or custom tools. Do not build a workflow merely because the capability is reusable by the agent: multiple independent tools do not become one workflow. Use `workflow-builder` before `build-agent` only when one agent tool call must execute an ordered multi-node procedure, or when the user explicitly asks for a workflow that is reusable or callable outside the agent; then attach it via `workflowContext`. For questions about existing agents ("what agents do I have?") call `agents(action="list")` directly — no intent gate; and to edit an agent NOT built in this conversation, find its id via `agents(action="list")` and pass it as `agentId`. Pass `agentId` only when the user\'s intent is to edit that specific agent. A request to build/create a NEW agent never passes `agentId`: pass `name` with `createNew: true`, even if an agent with the same or a similar name already exists — duplicate names are allowed, and an existing agent must never be repurposed to satisfy a create request. Agents the request merely references — as sub-agents, delegation targets, or examples — are not the build target: forward those mentions inside `message` only, never as `agentId`.' : ''; const WORKFLOW_ROUTE_GATE_REF = isAgentFeatureEnabled() diff --git a/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts b/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts index 5b791f7ae2b..12b8a3b2980 100644 --- a/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts +++ b/packages/@n8n/instance-ai/src/runtime/run-state-registry.ts @@ -1,7 +1,7 @@ import type { InstanceAiThreadStatusResponse } from '@n8n/api-types'; import { nanoid } from 'nanoid'; -import type { InstanceAiTraceContext, ModelConfig } from '../types'; +import type { InstanceAiTraceContext, ModelConfig, OrchestrationContext } from '../types'; import type { InstanceAiLivenessPolicy, InstanceAiLivenessSurface, @@ -24,6 +24,10 @@ export interface ActiveRunState { export interface SuspendedRunState extends ActiveRunState { agentRunId: string; agent: unknown; + /** The orchestration context the agent's tools closed over. Stored so a + * resume can rebind `tracing` to the new resume trace — spans emitted + * through the suspended turn's shut-down runtime export nothing. */ + orchestrationContext?: OrchestrationContext; threadId: string; user: TUser; toolCallId: string; diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/build-agent.tool.test.ts b/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/build-agent.tool.test.ts index 0808918c595..3f1b339734e 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/build-agent.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/__tests__/build-agent.tool.test.ts @@ -552,6 +552,45 @@ describe('build-agent tool', () => { }); describe('deferred agentId-path binding', () => { + it('rejects foreign agentId when the passed name contradicts the resolved agent name', async () => { + const { context, delegate } = makeContext(); + vi.mocked(delegate.resolveAgentName).mockResolvedValue('Support Triage Agent'); + + const result = await runTool(context, { + message: 'Build Ops Companion', + agentId: 'agent-existing', + name: 'Ops Companion', + }); + + expect(result).toEqual({ + ok: false, + error: + 'Agent agent-existing is named "Support Triage Agent", but name "Ops Companion" was passed. ' + + 'To create a new agent named "Ops Companion", pass `name` only (no `agentId`). ' + + 'To edit "Support Triage Agent", pass `agentId` only and put any rename instruction in `message`.', + }); + expect(delegate.streamBuild).not.toHaveBeenCalled(); + expect(saveAgentBuilderTarget).not.toHaveBeenCalled(); + }); + + it('allows foreign agentId when the passed name matches the resolved agent name', async () => { + const { context, delegate } = makeContext(); + vi.mocked(delegate.resolveAgentName).mockResolvedValue('Ops Companion'); + vi.mocked(delegate.streamBuild).mockResolvedValue(fakeStream([], 'Editing it.')); + + await runTool(context, { + message: 'Add a tool', + agentId: 'agent-existing', + name: ' ops companion ', + }); + + expect(delegate.streamBuild).toHaveBeenCalledWith( + 'agent-existing', + 'Add a tool', + expect.objectContaining({ threadId: 'ia-builder:thread-1:agent-existing' }), + ); + }); + it('does not persist the target when the agentId path fails before the stream settles', async () => { const { context, delegate } = makeContext(); vi.mocked(delegate.streamBuild).mockRejectedValue(new Error('agent:update forbidden')); @@ -989,6 +1028,53 @@ describe('build-agent tool', () => { modelConfig: context.modelId, }); }); + + it('creates a fresh agent instead of switching back when createNew is set and the name matches a session agent', async () => { + const { context, delegate } = makeContext(); + const boundTarget: AgentBuilderTarget = { + agentId: 'agent-1', + projectId: 'proj-1', + name: 'Platform Cycle Tracker', + }; + context.domainContext!.agentBuilderTarget = boundTarget; + vi.mocked(findSessionAgentByName).mockResolvedValue(boundTarget); + vi.mocked(delegate.createAgent).mockResolvedValue({ + agentId: 'agent-3', + projectId: 'proj-1', + }); + vi.mocked(delegate.streamBuild).mockResolvedValue(fakeStream([], 'Created it.')); + + await runTool(context, { + message: 'Build another tracker', + name: 'Platform Cycle Tracker', + createNew: true, + }); + + expect(findSessionAgentByName).not.toHaveBeenCalled(); + expect(delegate.createAgent).toHaveBeenCalledWith('Platform Cycle Tracker'); + expect(delegate.streamBuild).toHaveBeenCalledWith( + 'agent-3', + 'Build another tracker', + expect.objectContaining({ threadId: 'ia-builder:thread-1:agent-3' }), + ); + }); + + it.each([ + { agentId: 'agent-1', createNew: true, message: 'Build it' }, + { createNew: true, message: 'Build it' }, + ])('rejects createNew when combined with agentId or missing name', async (input) => { + const { context, delegate } = makeContext(); + + const result = await runTool(context, input); + + expect(result).toEqual({ + ok: false, + error: + 'createNew requires `name` and cannot be combined with `agentId` — pass `name` only to create a new agent.', + }); + expect(delegate.createAgent).not.toHaveBeenCalled(); + expect(delegate.streamBuild).not.toHaveBeenCalled(); + }); }); describe('interactive suspension cascade', () => { diff --git a/packages/@n8n/instance-ai/src/tools/orchestration/build-agent.tool.ts b/packages/@n8n/instance-ai/src/tools/orchestration/build-agent.tool.ts index 4060bf2908e..12ce5a80cd5 100644 --- a/packages/@n8n/instance-ai/src/tools/orchestration/build-agent.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/orchestration/build-agent.tool.ts @@ -158,7 +158,17 @@ const buildAgentInputSchema = z.object({ .describe( 'Agent name. A name matching an agent already built in this conversation switches back ' + 'to that agent; a new name creates a new agent and makes it the active target. Omit on ' + - 'follow-up calls for the current agent.', + 'follow-up calls for the current agent. Combine with `createNew: true` to force creating ' + + 'a fresh agent when the name matches one built earlier this conversation.', + ), + createNew: z + .boolean() + .optional() + .describe( + 'Set true when the user asks to create a brand-new agent. Bypasses the same-name ' + + 'switch-back: with createNew, `name` always creates a fresh agent even if that name ' + + 'matches one built earlier in this conversation. Requires `name`; never combine with ' + + '`agentId`. Omit for edits, follow-ups, and switch-backs.', ), agentId: z .string() @@ -166,7 +176,12 @@ const buildAgentInputSchema = z.object({ .describe( 'Existing agent id to edit — use the `agentId` returned by earlier build-agent ' + 'results. Pass to start editing that agent or to switch the active build target; ' + - 'omit on follow-up calls.', + 'omit on follow-up calls. Only pass this when the user explicitly wants to change ' + + 'that specific existing agent. NEVER pass it for a request to build/create a NEW ' + + 'agent — even if an agent with the same or a similar name already exists in the ' + + 'project (duplicate names are allowed). Agents the request merely references — as ' + + 'sub-agents, delegation targets, or examples — are not the build target: mention ' + + 'them in `message` instead.', ), workflowContext: z .array(z.object({ id: z.string(), name: z.string(), description: z.string().optional() })) @@ -616,9 +631,34 @@ type TargetResolution = | { ok: false; error: string }; const NO_TARGET_INPUT_ERROR = 'Pass name to create a new agent or agentId to edit an existing one.'; +const CREATE_NEW_INPUT_ERROR = + 'createNew requires `name` and cannot be combined with `agentId` — pass `name` only to create a new agent.'; const AGENT_ID_NEEDS_PROJECT_ERROR = 'Cannot bind to agentId without an active project context. Start this conversation from within a project.'; +function buildAgentTargetNameMismatchError( + agentId: string, + realName: string, + passedName: string, +): string { + return ( + `Agent ${agentId} is named "${realName}", but name "${passedName}" was passed. ` + + `To create a new agent named "${passedName}", pass \`name\` only (no \`agentId\`). ` + + `To edit "${realName}", pass \`agentId\` only and put any rename instruction in \`message\`.` + ); +} + +function rejectAgentTargetNameMismatch( + agentId: string, + realName: string | undefined, + passedName: string | undefined, +): TargetResolution | undefined { + if (!passedName || !realName || agentNamesMatch(passedName, realName)) { + return undefined; + } + return { ok: false, error: buildAgentTargetNameMismatchError(agentId, realName, passedName) }; +} + /** * Resolve which agent this call should build/edit. A bound target stays * active by default; passing `name` or `agentId` can create a new target or @@ -636,8 +676,14 @@ async function resolveTargetForCall( input: z.infer, boundTarget: AgentBuilderTarget | undefined, ): Promise { + if (input.createNew && (input.agentId || !input.name)) { + return { ok: false, error: CREATE_NEW_INPUT_ERROR }; + } + if (input.agentId) { if (boundTarget && input.agentId === boundTarget.agentId) { + const mismatch = rejectAgentTargetNameMismatch(input.agentId, boundTarget.name, input.name); + if (mismatch) return mismatch; return { ok: true, target: boundTarget, bindAfterTurn: false }; } if (!domainContext.projectId) { @@ -651,6 +697,8 @@ async function resolveTargetForCall( } catch { name = undefined; } + const mismatch = rejectAgentTargetNameMismatch(input.agentId, name, input.name); + if (mismatch) return mismatch; return { ok: true, target: { @@ -663,19 +711,21 @@ async function resolveTargetForCall( } if (input.name) { - // Guards against the orchestrator redundantly repeating `name` on a - // follow-up call for the agent already being built. - if (boundTarget && agentNamesMatch(input.name, boundTarget.name)) { - return { ok: true, target: boundTarget, bindAfterTurn: false }; - } - // A name matching an agent already built/targeted this conversation is a - // switch-back, not a creation — the duplicate-agent failure mode this - // registry exists to prevent. Deferred persist like the agentId path: the - // agent may have been deleted since, and a failed turn must not clobber - // the current binding. - const sessionAgent = await findSessionAgentByName(domainContext, input.name); - if (sessionAgent) { - return { ok: true, target: sessionAgent, bindAfterTurn: true }; + if (!input.createNew) { + // Guards against the orchestrator redundantly repeating `name` on a + // follow-up call for the agent already being built. + if (boundTarget && agentNamesMatch(input.name, boundTarget.name)) { + return { ok: true, target: boundTarget, bindAfterTurn: false }; + } + // A name matching an agent already built/targeted this conversation is a + // switch-back, not a creation — the duplicate-agent failure mode this + // registry exists to prevent. Deferred persist like the agentId path: the + // agent may have been deleted since, and a failed turn must not clobber + // the current binding. + const sessionAgent = await findSessionAgentByName(domainContext, input.name); + if (sessionAgent) { + return { ok: true, target: sessionAgent, bindAfterTurn: true }; + } } const created = await delegate.createAgent(input.name); const target: AgentBuilderTarget = { @@ -697,9 +747,12 @@ export function createBuildAgentTool(context: OrchestrationContext) { .description( 'Delegate agent building to the agents-module builder, running as a sub-agent. ' + 'Pass `name` to start a new agent or `agentId` to edit an existing one; calls ' + - 'without either keep editing the current agent. To build ANOTHER agent in the same ' + + 'without either keep editing the current agent. Create vs. edit follows user ' + + 'intent, not name collisions: a request to build a NEW agent always passes `name` ' + + 'plus `createNew: true` — never `agentId` — even when a same-named agent already exists in the ' + + 'project. To build ANOTHER agent in the same ' + 'conversation, pass its `name` or `agentId` — a name matching an agent already built ' + - 'in this conversation switches back to it; an unmatched name creates a new agent and ' + + 'in this conversation switches back to it (unless `createNew: true` is passed); an unmatched name creates a new agent and ' + 'switches the active target. The builder can also publish or unpublish the target ' + 'agent when the user asks to publish, activate, make it live/usable, or unpublish — ' + 'forward that intent in `message`; never tell the user to open the agent editor and ' + diff --git a/packages/@n8n/instance-ai/src/tools/tool-ids.ts b/packages/@n8n/instance-ai/src/tools/tool-ids.ts index c4b944c4672..9becc9cebe0 100644 --- a/packages/@n8n/instance-ai/src/tools/tool-ids.ts +++ b/packages/@n8n/instance-ai/src/tools/tool-ids.ts @@ -1,3 +1,5 @@ +import { isAgentFeatureEnabled } from '../utils/agent-feature-enabled'; + export const DOMAIN_TOOL_IDS = { WORKFLOWS: 'workflows', EVALS: 'evals', @@ -62,6 +64,10 @@ export const ALWAYS_LOADED_TOOL_NAMES = new Set([ DOMAIN_TOOL_IDS.AGENTS, 'web-search', 'fetch-url', + // build-agent is the primary route for agent-anchored intents; deferring it + // costs 2 LLM rounds (search_tools + load_tool) and a prompt-cache rewrite + // on every agent build. + ...(isAgentFeatureEnabled() ? [ORCHESTRATION_TOOL_IDS.BUILD_AGENT] : []), ]); export const CHECKPOINT_FOLLOW_UP_TOOL_NAMES = new Set([ diff --git a/packages/cli/src/controllers/__tests__/ai.controller.test.ts b/packages/cli/src/controllers/__tests__/ai.controller.test.ts index 97c6d6e881e..b76aec3be15 100644 --- a/packages/cli/src/controllers/__tests__/ai.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/ai.controller.test.ts @@ -14,19 +14,20 @@ import type { AiGatewayService } from '@/services/ai-gateway.service'; import type { AiUsageService } from '@/services/ai-usage.service'; import type { WorkflowBuilderService } from '@/services/ai-workflow-builder.service'; import type { AiService } from '@/services/ai.service'; +import type { FreeAiCreditsService } from '@/services/free-ai-credits.service'; import { AiController, type FlushableResponse } from '../ai.controller'; describe('AiController', () => { const aiService = mock(); const workflowBuilderService = mock(); + const freeAiCreditsService = mock(); const aiUsageService = mock(); const aiGatewayService = mock(); const controller = new AiController( aiService, workflowBuilderService, - mock(), - mock(), + freeAiCreditsService, aiUsageService, aiGatewayService, ); diff --git a/packages/cli/src/controllers/ai.controller.ts b/packages/cli/src/controllers/ai.controller.ts index 61b8c9184f2..ffe83cb112b 100644 --- a/packages/cli/src/controllers/ai.controller.ts +++ b/packages/cli/src/controllers/ai.controller.ts @@ -1,8 +1,4 @@ -import type { - AiGatewayConfigDto, - AiGatewayUsageResponse, - CreateCredentialDto, -} from '@n8n/api-types'; +import type { AiGatewayConfigDto, AiGatewayUsageResponse } from '@n8n/api-types'; import { AiChatRequestDto, AiApplySuggestionRequestDto, @@ -19,12 +15,10 @@ import { AuthenticatedRequest } from '@n8n/db'; import { Body, Get, Licensed, Post, Query, RestController, GlobalScope } from '@n8n/decorators'; import { type AiAssistantSDK, APIResponseError } from '@n8n_io/ai-assistant-sdk'; import { Response } from 'express'; -import { OPEN_AI_API_CREDENTIAL_TYPE } from 'n8n-workflow'; import { strict as assert } from 'node:assert'; import { WritableStream } from 'node:stream/web'; -import { FREE_AI_CREDITS_CREDENTIAL_NAME, STREAM_SEPARATOR } from '@/constants'; -import { CredentialsService } from '@/credentials/credentials.service'; +import { STREAM_SEPARATOR } from '@/constants'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ContentTooLargeError } from '@/errors/response-errors/content-too-large.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -34,7 +28,7 @@ import { AiGatewayService } from '@/services/ai-gateway.service'; import { AiUsageService } from '@/services/ai-usage.service'; import { WorkflowBuilderService } from '@/services/ai-workflow-builder.service'; import { AiService } from '@/services/ai.service'; -import { UserService } from '@/services/user.service'; +import { FreeAiCreditsService } from '@/services/free-ai-credits.service'; export type FlushableResponse = Response & { flush: () => void }; @@ -43,8 +37,7 @@ export class AiController { constructor( private readonly aiService: AiService, private readonly workflowBuilderService: WorkflowBuilderService, - private readonly credentialsService: CredentialsService, - private readonly userService: UserService, + private readonly freeAiCreditsService: FreeAiCreditsService, private readonly aiUsageService: AiUsageService, private readonly aiGatewayService: AiGatewayService, ) {} @@ -225,28 +218,7 @@ export class AiController { @Post('/free-credits') async aiCredits(req: AuthenticatedRequest, _: Response, @Body payload: AiFreeCreditsRequestDto) { try { - const aiCredits = await this.aiService.createFreeAiCredits(req.user); - - const credentialProperties: CreateCredentialDto = { - name: FREE_AI_CREDITS_CREDENTIAL_NAME, - type: OPEN_AI_API_CREDENTIAL_TYPE, - data: { - apiKey: aiCredits.apiKey, - url: aiCredits.url, - }, - projectId: payload?.projectId, - }; - - const newCredential = await this.credentialsService.createManagedCredential( - credentialProperties, - req.user, - ); - - await this.userService.updateSettings(req.user.id, { - userClaimedAiCredits: true, - }); - - return newCredential; + return await this.freeAiCreditsService.claim(req.user, payload?.projectId); } catch (e) { assert(e instanceof Error); throw new InternalServerError(e.message, e); diff --git a/packages/cli/src/modules/agents/__tests__/agent-integration-persistence.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-integration-persistence.service.test.ts index 9174784b1ef..30ad8be0f45 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-integration-persistence.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-integration-persistence.service.test.ts @@ -101,6 +101,23 @@ describe('AgentIntegrationPersistenceService', () => { ); }); + it('consumes a same-type draft entry (empty credentialId) when connecting a real credential', async () => { + const { service, agentRepository, chatIntegrationService, runtimeCacheService } = makeService(); + const agent = makeAgent({ integrations: [{ type: 'slack', credentialId: '' }] }); + + await service.saveCredentialIntegration(agent, { type: 'slack', credentialId: 'c1' }); + + expect(agent.integrations).toEqual([{ type: 'slack', credentialId: 'c1' }]); + expect(agent.versionId).not.toBe(agent.activeVersionId); + expect(runtimeCacheService.clearRuntimes).toHaveBeenCalledWith(agentId); + expect(agentRepository.save).toHaveBeenCalledWith(agent); + expect(chatIntegrationService.broadcastIntegrationChange).toHaveBeenCalledWith( + agentId, + { type: 'slack', credentialId: 'c1' }, + 'connect', + ); + }); + it('appends new credential integrations while preserving existing siblings', async () => { const { service, agentRepository, chatIntegrationService, runtimeCacheService } = makeService(); const agent = makeAgent({ 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 f442ec413fa..025e9fc5d57 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 @@ -314,7 +314,7 @@ describe('AgentIntegrationsController integration credentials', () => { 'project-1', { id: 'user-1' }, undefined, - { syncIntegrations: false }, + { syncIntegrations: false, ignoreDraftIntegrations: true }, ); expect(chatIntegrationService.connect).toHaveBeenCalledWith( 'agent-1', @@ -427,7 +427,7 @@ describe('AgentIntegrationsController integration credentials', () => { 'project-1', { id: 'user-1' }, undefined, - { syncIntegrations: false }, + { syncIntegrations: false, ignoreDraftIntegrations: true }, ); expect(chatIntegrationService.connect).toHaveBeenCalledWith( 'agent-1', @@ -571,6 +571,25 @@ describe('AgentIntegrationsController integration credentials', () => { }); }); + it('reports a draft integration (empty credentialId) as disconnected', async () => { + const agentRepository = mock(); + agentRepository.findByIdAndProjectId.mockResolvedValue({ + id: 'agent-1', + projectId: 'project-1', + integrations: [{ type: 'slack', credentialId: '' }], + } as never); + + const { controller } = makeController({ agentRepository }); + + await expect( + controller.integrationStatus( + { params: { projectId: 'project-1' } } as never, + undefined as never, + 'agent-1', + ), + ).resolves.toEqual({ status: 'disconnected', integrations: [] }); + }); + it('disconnects the channel before removing the persisted integration', async () => { const agentRepository = mock(); const agent = { diff --git a/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts index 9dfa8001d42..83dfe02d3eb 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts @@ -486,6 +486,30 @@ describe('AgentPublishService', () => { ); }); + it('ignores other draft integrations when connecting a channel (ignoreDraftIntegrations)', async () => { + const { service, agentRepository, agentValidationService } = makeService(); + const integrations = [ + { type: 'slack', credentialId: 'slack-1' }, + { type: 'telegram', credentialId: '' }, + ]; + const agent = makeAgent({ integrations: integrations as never }); + agentRepository.findByIdAndProjectId.mockResolvedValue(agent); + + await service.publishAgent(agentId, projectId, user, undefined, { + ignoreDraftIntegrations: true, + }); + + expect(agentValidationService.validateAgentEntityConfiguration).toHaveBeenCalledWith( + agent, + projectId, + expect.anything(), + expect.anything(), + 'publish', + [{ type: 'slack', credentialId: 'slack-1' }], + ); + expect(agent.integrations).toBe(integrations); + }); + it('maps publish history rows and marks the active version', async () => { const { service, agentRepository, agentHistoryRepository } = makeService(); const active = makeHistory({ versionId: 'active-version', author: 'Ada Lovelace' }); 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 5f8401ff517..75d76a37bbc 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 @@ -20,6 +20,8 @@ import type { CredentialTypes } from '@/credential-types'; import type { McpRegistryService } from '@/modules/mcp-registry/registry/mcp-registry.service'; import type { NodeTypes } from '@/node-types'; import type { DynamicNodeParametersService } from '@/services/dynamic-node-parameters.service'; +import type { FreeAiCreditsService } from '@/services/free-ai-credits.service'; +import type { Telemetry } from '@/telemetry'; import type { AgentConfigService } from '../agent-config.service'; import type { AgentCustomToolsService } from '../agent-custom-tools.service'; @@ -27,6 +29,7 @@ import type { AgentIntegrationPersistenceService } from '../agent-integration-pe import type { AgentPublishService } from '../agent-publish.service'; import type { AgentSkillsService } from '../agent-skills.service'; import type { AgentTaskService } from '../agent-task.service'; +import type { AgentValidationService } from '../agent-validation.service'; import type { AgentsToolsService } from '../agents-tools.service'; import type { AgentsService } from '../agents.service'; import type { AttachableWorkflowsService } from '../attachable-workflows.service'; @@ -76,6 +79,11 @@ function makeService() { const mcpRegistryService = mock(); const agentTaskService = mock(); const agentPublishService = mock(); + const agentValidationService = mock(); + agentValidationService.validateAgentConfiguration.mockResolvedValue({ + status: 'valid', + issues: [], + }); const aiService = mock(); aiService.isProxyEnabled.mockReturnValue(false); const dynamicNodeParametersService = mock(); @@ -111,6 +119,9 @@ function makeService() { nodeTypes, mock({ enabled: true }), mock(), + mock(), + mock(), + agentValidationService, ); return { @@ -120,6 +131,7 @@ function makeService() { attachableWorkflowsService, agentTaskService, agentPublishService, + agentValidationService, nodeTypes, outboundHttp, }; @@ -245,6 +257,15 @@ describe('AgentsBuilderToolsService', () => { expect(toolNames).toContain(BUILDER_TOOLS.RESOLVE_INTEGRATION); }); + it('registers the finish_setup interactive tool in the builder toolset', () => { + const { service } = makeService(); + + const toolNames = service + .getTools(agentId, projectId, credentialProvider, user) + .json.map((tool) => tool.name); + expect(toolNames).toContain(BUILDER_TOOLS.FINISH_SETUP); + }); + it('registers publish and unpublish tools in the builder toolset', () => { const { service } = makeService(); @@ -276,6 +297,49 @@ describe('AgentsBuilderToolsService', () => { config: { ...baseConfig, integrations: [] }, configHash: getAgentConfigHash({ ...baseConfig, integrations: [] }), }); + expect(result).not.toHaveProperty('configMutated'); + }); + + it('write_config success result carries configMutated and agentId', async () => { + const { service, agentsService } = makeService(); + const currentConfig = { ...baseConfig, integrations: [] }; + const updatedConfig = { ...currentConfig, instructions: 'Help with support tickets.' }; + const normalizedConfig = { + ...updatedConfig, + config: { webSearch: { enabled: true }, promptCaching: { enabled: true } }, + }; + agentsService.findById.mockResolvedValue(makeAgent(baseConfig)); + agentsService.updateConfig.mockResolvedValue({ + config: normalizedConfig, + updatedAt: '2026-01-02T00:00:00.000Z', + versionId: 'v2', + }); + + const result = await getJsonTool(service, BUILDER_TOOLS.WRITE_CONFIG).handler!( + { + baseConfigHash: getAgentConfigHash(currentConfig), + json: JSON.stringify(updatedConfig), + }, + ctx, + ); + + expect(result).toEqual({ ok: true, configMutated: true, agentId }); + }); + + it('write_config failure result is not stamped with configMutated', async () => { + const { service, agentsService } = makeService(); + agentsService.findById.mockResolvedValue(makeAgent(baseConfig)); + + const result = await getJsonTool(service, BUILDER_TOOLS.WRITE_CONFIG).handler!( + { + baseConfigHash: 'stale-hash', + json: JSON.stringify(baseConfig), + }, + ctx, + ); + + expect(result).toEqual(expect.objectContaining({ ok: false })); + expect(result).not.toHaveProperty('configMutated'); }); it('list_integration_types returns builder guidance for integration versus node-tool choice', async () => { @@ -383,7 +447,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(agentsService.updateConfig).toHaveBeenCalledWith(agentId, projectId, normalizedConfig); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); it('patch_config rejects stale baseConfigHash without updating or echoing the config', async () => { @@ -452,7 +516,7 @@ describe('AgentsBuilderToolsService', () => { integrations: [currentIntegrations[0], currentIntegrations[2]], }), ); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); it('patch_config strips legacy schedule integrations from the current snapshot', async () => { @@ -522,7 +586,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(agentsService.updateConfig).toHaveBeenCalledWith(agentId, projectId, normalizedConfig); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); it('write_config strips legacy schedule integrations before saving', async () => { @@ -680,7 +744,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(agentsService.updateConfig).toHaveBeenCalledWith(agentId, projectId, normalizedConfig); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); it('patch_config allows $fromAI on runtime fields when dynamic selectors are fixed', async () => { @@ -723,7 +787,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(agentsService.updateConfig).toHaveBeenCalledWith(agentId, projectId, normalizedConfig); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); it('write_config allows unrelated edits when an existing dynamic selector already uses $fromAI', async () => { @@ -760,7 +824,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(agentsService.updateConfig).toHaveBeenCalledWith(agentId, projectId, normalizedConfig); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); it('patch_config allows unrelated edits when an existing dynamic selector already uses $fromAI', async () => { @@ -799,7 +863,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(agentsService.updateConfig).toHaveBeenCalledWith(agentId, projectId, normalizedConfig); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, configMutated: true, agentId }); }); // Native web-search provider-tool derivation (add defaults, fill missing @@ -873,7 +937,61 @@ describe('AgentsBuilderToolsService', () => { expect(agentsService.updateConfig).not.toHaveBeenCalled(); }); - it('write_config rejects draft LLM config without updating', async () => { + it('write_config accepts a draft config without model and credential', async () => { + const { service, agentsService } = makeService(); + const { credential: _credential, ...draftBase } = baseConfig; + const currentDraftConfig = { ...draftBase, model: '', integrations: [] }; + const draftConfig = { ...currentDraftConfig, credential: undefined }; + agentsService.findById.mockResolvedValue( + makeAgent({ ...draftBase, model: '' } as AgentJsonConfig), + ); + agentsService.updateConfig.mockResolvedValue({ + config: { ...currentDraftConfig, model: '' }, + updatedAt: '2026-01-02T00:00:00.000Z', + versionId: 'v2', + }); + + const result = await getJsonTool(service, BUILDER_TOOLS.WRITE_CONFIG).handler!( + { + baseConfigHash: getAgentConfigHash(currentDraftConfig), + json: JSON.stringify(draftConfig), + }, + ctx, + ); + + expect(result).toEqual({ ok: true, configMutated: true, agentId }); + expect(agentsService.updateConfig).toHaveBeenCalledWith( + agentId, + projectId, + expect.objectContaining({ model: '', instructions: 'Help the user.' }), + ); + }); + + it('write_config still rejects empty instructions on a draft', async () => { + const { service, agentsService } = makeService(); + const { credential: _credential, ...draftBase } = baseConfig; + const currentDraftConfig = { ...draftBase, model: '', integrations: [] }; + const draftConfig = { ...currentDraftConfig, instructions: '' }; + agentsService.findById.mockResolvedValue( + makeAgent({ ...draftBase, model: '' } as AgentJsonConfig), + ); + + const result = await getJsonTool(service, BUILDER_TOOLS.WRITE_CONFIG).handler!( + { + baseConfigHash: getAgentConfigHash(currentDraftConfig), + json: JSON.stringify(draftConfig), + }, + ctx, + ); + + expect(result).toEqual({ + ok: false, + errors: [expect.objectContaining({ path: '/instructions' })], + }); + expect(agentsService.updateConfig).not.toHaveBeenCalled(); + }); + + it('write_config rejects a draft payload when the agent already has a model', async () => { const { service, agentsService } = makeService(); const currentConfig = { ...baseConfig, integrations: [] }; const draftConfig = { ...currentConfig, model: '', credential: undefined }; @@ -887,7 +1005,6 @@ describe('AgentsBuilderToolsService', () => { ctx, ); - expect(agentsService.updateConfig).not.toHaveBeenCalled(); expect(result).toEqual({ ok: false, errors: expect.arrayContaining([ @@ -895,6 +1012,58 @@ describe('AgentsBuilderToolsService', () => { expect.objectContaining({ path: 'credential' }), ]), }); + expect(agentsService.updateConfig).not.toHaveBeenCalled(); + }); + + it('patch_config succeeds on a draft config without model and credential', async () => { + const { service, agentsService } = makeService(); + const { credential: _credential, ...noCredential } = baseConfig; + const draftBase = { ...noCredential, model: '' }; + const currentConfig = { ...draftBase, integrations: [] }; + agentsService.findById.mockResolvedValue(makeAgent(draftBase as AgentJsonConfig)); + agentsService.updateConfig.mockResolvedValue({ + config: { ...currentConfig, instructions: 'Triage Slack messages.' }, + updatedAt: '2026-01-02T00:00:00.000Z', + versionId: 'v2', + }); + + const result = await getJsonTool(service, BUILDER_TOOLS.PATCH_CONFIG).handler!( + { + baseConfigHash: getAgentConfigHash(currentConfig), + operations: JSON.stringify([ + { op: 'replace', path: '/instructions', value: 'Triage Slack messages.' }, + ]), + }, + ctx, + ); + + expect(result).toEqual({ ok: true, configMutated: true, agentId }); + expect(agentsService.updateConfig).toHaveBeenCalledWith( + agentId, + projectId, + expect.objectContaining({ model: '', instructions: 'Triage Slack messages.' }), + ); + }); + + it('patch_config rejects clearing /model on a configured agent', async () => { + const { service, agentsService } = makeService(); + const currentConfig = { ...baseConfig, integrations: [] }; + agentsService.findById.mockResolvedValue(makeAgent(baseConfig)); + + const result = await getJsonTool(service, BUILDER_TOOLS.PATCH_CONFIG).handler!( + { + baseConfigHash: getAgentConfigHash(currentConfig), + operations: JSON.stringify([{ op: 'replace', path: '/model', value: '' }]), + }, + ctx, + ); + + expect(result).toEqual({ + ok: false, + stage: 'schema', + errors: expect.arrayContaining([expect.objectContaining({ path: 'model' })]), + }); + expect(agentsService.updateConfig).not.toHaveBeenCalled(); }); it('write_config rejects stale baseConfigHash without updating or echoing the config', async () => { @@ -1350,7 +1519,7 @@ describe('AgentsBuilderToolsService', () => { 'Pass every task you currently know how to write in one `tasks` array', ); expect(tool.description).toContain('config.tasks'); - expect(tool.description).toContain('{ ok: true, tasks:'); + expect(tool.description).toContain('{ ok: true, configMutated: true, agentId, tasks:'); expect(tool.description).toContain('{ ok: false, errors }'); }); @@ -1409,6 +1578,8 @@ describe('AgentsBuilderToolsService', () => { ]); expect(result).toEqual({ ok: true, + configMutated: true, + agentId, tasks: [ { id: 'task-1', name: taskOneInput.name, enabled: true }, { id: 'task-2', name: taskTwoInput.name, enabled: true }, @@ -1495,6 +1666,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(result).toEqual({ ok: true, + configMutated: true, agentId, activeVersionId: 'v-active', versionId: 'v-active', @@ -1521,6 +1693,7 @@ describe('AgentsBuilderToolsService', () => { ); expect(result).toEqual({ ok: true, + configMutated: true, agentId, activeVersionId: 'v-history', versionId: 'v-draft', @@ -1568,7 +1741,12 @@ describe('AgentsBuilderToolsService', () => { projectId, }); expect(agentPublishService.unpublishAgent).toHaveBeenCalledWith(agentId, projectId); - expect(result).toEqual({ ok: true, agentId, activeVersionId: null }); + expect(result).toEqual({ + ok: true, + configMutated: true, + agentId, + activeVersionId: null, + }); }); it('denies unpublish when the user lacks agent:unpublish', async () => { 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 7b66ab3e29a..e4ef26ed619 100644 --- a/packages/cli/src/modules/agents/agent-integration-persistence.service.ts +++ b/packages/cli/src/modules/agents/agent-integration-persistence.service.ts @@ -70,7 +70,12 @@ export class AgentIntegrationPersistenceService { throw new UserError('Credential integration requires a credential ID.'); } - const existing = agent.integrations ?? []; + // Drop a same-type draft entry (empty credentialId, written by the builder + // 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 === ''), + ); const alreadyExists = existing.some((i) => i.type === type && i.credentialId === credentialId); agent.integrations = alreadyExists diff --git a/packages/cli/src/modules/agents/agent-integrations.controller.ts b/packages/cli/src/modules/agents/agent-integrations.controller.ts index a337231da1a..a3c9abc1de9 100644 --- a/packages/cli/src/modules/agents/agent-integrations.controller.ts +++ b/packages/cli/src/modules/agents/agent-integrations.controller.ts @@ -82,7 +82,7 @@ export class AgentIntegrationsController { agent.projectId, req.user, undefined, - { syncIntegrations: false }, + { syncIntegrations: false, ignoreDraftIntegrations: true }, ); await this.chatIntegrationService.connect(agentId, integration, agent.projectId); await this.chatIntegrationService.broadcastIntegrationChange(agentId, integration, 'connect'); @@ -215,11 +215,17 @@ export class AgentIntegrationsController { const agent = await this.agentRepository.findByIdAndProjectId(agentId, req.params.projectId); if (!agent) throw new NotFoundError(`Agent "${agentId}" not found`); - const chatIntegrations = (agent.integrations ?? []).map((i) => ({ - type: i.type, - credentialId: i.credentialId, - ...('settings' in i ? { settings: i.settings } : {}), - })); + // Draft entries (`credentialId: ''`) written during the initial build so + // the panel can show a needs-setup chip aren't a real connection — report + // 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 !== '') + .map((i) => ({ + type: i.type, + credentialId: i.credentialId, + ...('settings' in i ? { settings: i.settings } : {}), + })); return { status: chatIntegrations.length > 0 ? 'connected' : 'disconnected', integrations: chatIntegrations, diff --git a/packages/cli/src/modules/agents/agent-publish.service.ts b/packages/cli/src/modules/agents/agent-publish.service.ts index 2988f5c3a4e..e9e147253fe 100644 --- a/packages/cli/src/modules/agents/agent-publish.service.ts +++ b/packages/cli/src/modules/agents/agent-publish.service.ts @@ -32,6 +32,13 @@ import { SubAgentCleanupService } from './sub-agents/sub-agent-cleanup.service'; export interface PublishAgentOptions { syncIntegrations?: boolean; + /** + * Validate as if not-yet-connected draft integrations (`credentialId: ''`) + * didn't exist. Connect-time publishes (connecting one of several + * drafted channels) pass this so another channel's still-unresolved + * draft doesn't block publishing the one currently being connected. + */ + ignoreDraftIntegrations?: boolean; } export type ValidAgentConfigValidationResponse = AgentConfigValidationResponse & { @@ -110,7 +117,14 @@ export class AgentPublishService { (await this.agentTaskRepository.findByAgentId(agentId)).map((task) => [task.id, task]), ); - const validation = await this.assertPublishable(agent, projectId, user, tasks, targetHistory); + const validation = await this.assertPublishable( + agent, + projectId, + user, + tasks, + targetHistory, + options.ignoreDraftIntegrations, + ); await this.agentRepository.manager.transaction(async (trx) => { if (targetHistory) { @@ -178,6 +192,7 @@ export class AgentPublishService { user: User, tasks: ReadonlyMap, targetHistory?: AgentHistory, + ignoreDraftIntegrations?: boolean, ): Promise { const credentialProvider = new AgentsCredentialProvider( this.credentialsService, @@ -185,20 +200,34 @@ export class AgentPublishService { user, ); + const baseIntegrations = agent.integrations ?? []; + const integrations = ignoreDraftIntegrations + ? baseIntegrations.filter((integration) => integration.credentialId !== '') + : baseIntegrations; + const validation = targetHistory ? await this.agentValidationService.validateAgentHistoryConfiguration( agent.id, projectId, targetHistory, - agent.integrations ?? [], + integrations, credentialProvider, ) - : await this.agentValidationService.validateAgentEntityConfiguration( - agent, - projectId, - tasks, - credentialProvider, - ); + : ignoreDraftIntegrations + ? await this.agentValidationService.validateAgentEntityConfiguration( + agent, + projectId, + tasks, + credentialProvider, + 'publish', + integrations, + ) + : await this.agentValidationService.validateAgentEntityConfiguration( + agent, + projectId, + tasks, + credentialProvider, + ); requireValidValidation(validation); return validation; diff --git a/packages/cli/src/modules/agents/agent-validation.service.ts b/packages/cli/src/modules/agents/agent-validation.service.ts index 73df2d453df..c0a8f3d0e1b 100644 --- a/packages/cli/src/modules/agents/agent-validation.service.ts +++ b/packages/cli/src/modules/agents/agent-validation.service.ts @@ -161,6 +161,13 @@ export class AgentValidationService { tasks: ReadonlyMap, credentialProvider: CredentialProvider, scope: AgentValidationScope = 'publish', + /** + * Validate against these integrations instead of `agent.integrations`. + * Used by connect-time publishes to exclude not-yet-connected draft + * entries (`credentialId: ''`) that would otherwise block publishing + * the channel currently being connected. + */ + integrationsOverride?: AgentIntegrationConfig[], ): Promise { return await this.runValidation( { @@ -169,7 +176,7 @@ export class AgentValidationService { config: agent.schema as unknown as AgentJsonConfig | null, skills: agent.skills ?? {}, customTools: agent.tools ?? {}, - integrations: agent.integrations ?? [], + integrations: integrationsOverride ?? agent.integrations ?? [], tasks, credentialProvider, }, diff --git a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts index bbcd2f6c752..e5887b08422 100644 --- a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts +++ b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts @@ -119,27 +119,16 @@ describe('builder model recommendations', () => { expect(prompt).toContain('## Example flows'); expect(prompt).toContain('## Response Style'); expect(prompt).not.toContain('## Builder runtime skills'); - expect(prompt).toContain('agent-builder-integrations'); + expect(prompt).toContain('agent-builder-external-services'); expect(prompt).toContain('agent-builder-memory'); - expect(prompt).toContain('agent-builder-node-tools'); expect(prompt).toContain('agent-builder-custom-tools'); expect(prompt).not.toContain('agent-builder-config-mutation'); expect(prompt).not.toContain('agent-builder-llm-selection'); - const nodeToolsSkill = getBuilderRuntimeSkills().find( - (s) => s.id === 'agent-builder-node-tools', - ); - expect(nodeToolsSkill?.instructions).toContain('agent-builder-resource-locators'); - }); - - it('does not tell the builder to write target agent descriptions', () => { - const prompt = buildPrompt(null); - - expect(prompt).not.toContain('Fresh agents must include a brief `description`'); - expect(prompt).toContain('Requires `name`, `model`, `credential`, and `instructions`'); - expect(prompt).not.toContain( - '"description": "Answers support questions and helps triage customer issues."', + const externalServicesSkill = getBuilderRuntimeSkills().find( + (s) => s.id === 'agent-builder-external-services', ); + expect(externalServicesSkill?.instructions).toContain('agent-builder-resource-locators'); }); it('routes subagent delegation to the sub-agent builder skill', () => { @@ -197,7 +186,7 @@ describe('builder model recommendations', () => { it('keeps always-on interaction and workflow guidance in the main prompt, deferring expressions to a skill', () => { const prompt = buildPrompt('### Recommended LLM Models\n\n- OpenAI: `openai/gpt-5` GPT-5'); - const skill = getBuilderRuntimeSkills().find((s) => s.id === 'agent-builder-node-tools'); + const skill = getBuilderRuntimeSkills().find((s) => s.id === 'agent-builder-external-services'); expect(prompt).toContain('### Recommended LLM Models'); expect(prompt).toContain('Never call two interactive tools in parallel'); @@ -224,10 +213,8 @@ describe('builder model recommendations', () => { expect(skills.map((skill) => skill.id)).toEqual([ 'agent-builder-custom-tools', - 'agent-builder-integrations', - 'agent-builder-mcp', + 'agent-builder-external-services', 'agent-builder-memory', - 'agent-builder-node-tools', 'agent-builder-resource-locators', 'agent-builder-sub-agents', 'agent-builder-target-skills', @@ -235,12 +222,12 @@ describe('builder model recommendations', () => { ]); expect(skillsById.has('agent-builder-research')).toBe(false); - const integrations = skillsById.get('agent-builder-integrations'); - expect(integrations?.description).toContain( + const externalServices = skillsById.get('agent-builder-external-services'); + expect(externalServices?.description).toContain( 'chat integration/trigger versus an MCP, node, or workflow tool', ); - expect(integrations?.instructions).toContain('Integration vs Callable Tool Decision'); - expect(integrations?.instructions).toContain('Linear callable tools'); + expect(externalServices?.instructions).toContain('Integration vs Callable Tool Decision'); + expect(externalServices?.instructions).toContain('Linear callable tools'); const resourceLocators = skillsById.get('agent-builder-resource-locators'); expect(resourceLocators?.description).toContain('stable dynamic selector fields'); @@ -249,11 +236,11 @@ describe('builder model recommendations', () => { }); it('does not tell the builder to prefer Slack OAuth credentials for chat integrations', () => { - const integrationsSkill = getBuilderRuntimeSkills().find( - (skill) => skill.id === 'agent-builder-integrations', + const externalServicesSkill = getBuilderRuntimeSkills().find( + (skill) => skill.id === 'agent-builder-external-services', ); - expect(integrationsSkill?.instructions).not.toContain('slackOAuth2Api'); - expect(integrationsSkill?.instructions).not.toContain('prefer the OAuth variant'); + expect(externalServicesSkill?.instructions).not.toContain('slackOAuth2Api'); + expect(externalServicesSkill?.instructions).not.toContain('prefer the OAuth variant'); }); }); diff --git a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts deleted file mode 100644 index 15e8f335275..00000000000 --- a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts +++ /dev/null @@ -1,320 +0,0 @@ -import { SUB_AGENT_MAX_CHILDREN_MAX, SUB_AGENT_MAX_CHILDREN_MIN } from '@n8n/api-types'; - -import { - FEW_SHOT_FLOWS_SECTION, - INTERACTIVE_TOOLS_SECTION, - READ_CONFIG_FRESHNESS_SECTION, - WORKFLOW_SECTION, - buildBuilderPrompt, -} from '../agents-builder-prompts'; -import { getConfigMutationPrompt } from '../prompts/config-mutation.prompt'; -import { getLlmSelectionPrompt } from '../prompts/llm-selection.prompt'; -import { TOOLS_PROMPT } from '../prompts/tools.prompt'; -import { getBuilderRuntimeSkills } from '../skills'; - -function normalizeWhitespace(value: string | undefined): string { - return value?.replace(/\s+/g, ' ').trim() ?? ''; -} - -describe('builder prompt stability', () => { - it('omits stale agent state while retaining config freshness guidance', () => { - const prompt = buildBuilderPrompt({ - agentPreviewPath: '/projects/project-1/agents/agent-1/preview', - modelRecommendationsSection: null, - }); - - expect(prompt).not.toContain('## Current Agent Config'); - expect(prompt).not.toContain('\n## Custom Tools\n'); - expect(prompt).not.toContain('## Builder runtime skills'); - expect(prompt).not.toContain('## Important'); - expect(prompt).toContain( - 'Always call `read_config` first whenever a request touches the config', - ); - }); -}); - -describe('create_skills / create_tasks batching guidance', () => { - it('names only the plural batch tools, not the old singular ones', () => { - const prompt = buildBuilderPrompt({ - agentPreviewPath: '/projects/project-1/agents/agent-1/preview', - modelRecommendationsSection: null, - }); - - expect(prompt).toContain('`create_skills`'); - expect(prompt).toContain('`create_tasks`'); - expect(prompt).not.toContain('`create_skill`'); - expect(prompt).not.toContain('`create_task`'); - - for (const skill of getBuilderRuntimeSkills()) { - expect(skill.allowedTools ?? []).not.toContain('create_skill'); - expect(skill.allowedTools ?? []).not.toContain('create_task'); - expect(skill.instructions).not.toContain('`create_skill`'); - expect(skill.instructions).not.toContain('`create_task`'); - } - }); - - it('mandates one call per fully-specified batch, not one call per item', () => { - const skills = getBuilderRuntimeSkills(); - const targetSkills = skills.find((skill) => skill.id === 'agent-builder-target-skills'); - const targetTasks = skills.find((skill) => skill.id === 'agent-builder-target-tasks'); - - expect(targetSkills?.instructions).toContain( - 'do not spread multiple fully-specified skills\n across separate calls', - ); - expect(targetTasks?.instructions).toContain( - 'do not spread multiple fully-specified tasks\n across separate calls', - ); - }); - - it('preserves the skill-attachment and task-publish rules for the batched tools', () => { - const skills = getBuilderRuntimeSkills(); - const targetSkills = skills.find((skill) => skill.id === 'agent-builder-target-skills'); - const targetTasks = skills.find((skill) => skill.id === 'agent-builder-target-tasks'); - - expect(targetSkills?.instructions).toContain( - 'Use `patch_config` or `write_config` to add a `{ "type": "skill", "id": "" }`', - ); - expect(targetTasks?.instructions).toContain( - '`create_tasks` adds a `{ type: "task", id, enabled }` ref per task to', - ); - expect(targetTasks?.instructions).toContain( - 'only start running once the agent is (re)published', - ); - }); - - it('permits create_skills and create_tasks together in one turn, never with an interactive tool or config mutation', () => { - expect(WORKFLOW_SECTION).toContain( - 'call `create_skills`\n and `create_tasks` in the same assistant response', - ); - expect(WORKFLOW_SECTION).toContain( - 'Do not combine either\n with an interactive tool or `write_config`/`patch_config` in that response.', - ); - }); -}); - -describe('compact tool output guidance', () => { - it('requires a fresh read_config before retrying a stale write/patch', () => { - expect(READ_CONFIG_FRESHNESS_SECTION).toContain( - '`patch_config` returns `stage: "stale"`, call `read_config` and retry once\nusing the `config` and `configHash` it returns.', - ); - - const mutationPrompt = getConfigMutationPrompt(); - expect(mutationPrompt).toContain('Follow Config Freshness'); - expect(mutationPrompt).not.toContain('retry once using the'); - }); - - it('never claims a stale or mutation-success response carries the config or configHash', () => { - expect(READ_CONFIG_FRESHNESS_SECTION).toContain( - '`read_config` is the only tool that returns the full `config`. A successful\n`write_config`/`patch_config` returns only `{ ok: true }` as confirmation\n— never the config, its hash, timestamps, or version — so it cannot serve as\na `baseConfigHash` for a later write.', - ); - expect(READ_CONFIG_FRESHNESS_SECTION).not.toContain('configHash`, `updatedAt`'); - expect(READ_CONFIG_FRESHNESS_SECTION).not.toContain('retry once\nfrom the returned'); - - const mutationPrompt = getConfigMutationPrompt(); - expect(mutationPrompt).not.toContain('never echo the config back'); - expect(mutationPrompt).not.toContain('retry once from the returned'); - }); - - it('requires an immediately preceding read_config before every later mutation', () => { - expect(READ_CONFIG_FRESHNESS_SECTION).toContain( - 'Call `read_config`\nagain immediately before every later mutation and before any later\ninspection of the config.', - ); - }); -}); - -describe('prompt-caching rule dedup', () => { - it('states the detailed mandatory prompt-caching rule once, in Agent Config Rules', () => { - const prompt = buildBuilderPrompt({ - agentPreviewPath: '/projects/project-1/agents/agent-1/preview', - modelRecommendationsSection: null, - }); - - const detailedRule = 'this is mandatory and must\n never be disabled'; - expect(prompt.split(detailedRule)).toHaveLength(2); - - const rulesIndex = prompt.indexOf('#### Agent Config Rules'); - const ruleIndex = prompt.indexOf(detailedRule); - expect(rulesIndex).toBeGreaterThan(-1); - expect(ruleIndex).toBeGreaterThan(rulesIndex); - }); -}); - -describe('agents builder integrations prompt', () => { - it('does not tell the builder to prefer Slack OAuth credentials for chat integrations', () => { - const integrationsSkill = getBuilderRuntimeSkills().find( - (skill) => skill.id === 'agent-builder-integrations', - ); - - expect(integrationsSkill?.instructions).not.toContain('slackOAuth2Api'); - expect(integrationsSkill?.instructions).not.toContain('prefer the OAuth variant'); - }); -}); - -describe('chat-channel credential guidance', () => { - it('mandates configure_channel and forbids ask_credential for chat-channel credentials', () => { - expect(INTERACTIVE_TOOLS_SECTION).toContain( - '`ask_credential` for node-tool, MCP-server, and fallback web-search credentials', - ); - expect(normalizeWhitespace(INTERACTIVE_TOOLS_SECTION)).toContain( - "anything that isn't a node-tool credential, MCP-server credential, fallback web-search credential, or channel choice", - ); - expect(INTERACTIVE_TOOLS_SECTION).toContain( - 'NEVER use it for a chat-channel\n credential — use `configure_channel` instead.', - ); - expect(INTERACTIVE_TOOLS_SECTION).toContain( - '`configure_channel`: ALWAYS use this to connect a chat platform', - ); - - const integrationsSkill = getBuilderRuntimeSkills().find( - (skill) => skill.id === 'agent-builder-integrations', - ); - expect(integrationsSkill?.instructions).toContain( - 'ALWAYS use `configure_channel` for chat-channel\n credentials — never `ask_credential`', - ); - - const llmSelectionPrompt = getLlmSelectionPrompt(null); - expect(llmSelectionPrompt).toContain( - 'Use `ask_credential` for node tools, MCP servers, and fallback web-search credentials.', - ); - expect(llmSelectionPrompt).not.toContain( - 'Use `ask_credential` for node tools and integrations.', - ); - }); - - it('references ask_questions in the batching guidance', () => { - expect(WORKFLOW_SECTION).toContain('Clarify missing decisions through the Interactive tools'); - expect(INTERACTIVE_TOOLS_SECTION).toContain( - 'Batch every\n question you currently need into a single call', - ); - }); - - it('tells the builder how to remove an existing chat integration', () => { - const integrationsSkill = getBuilderRuntimeSkills().find( - (skill) => skill.id === 'agent-builder-integrations', - ); - - expect(integrationsSkill?.recommendedTools).toContain('ask_questions'); - expect(integrationsSkill?.allowedTools).toContain('ask_questions'); - expect(integrationsSkill?.instructions).toContain('To remove an existing chat integration'); - expect(integrationsSkill?.instructions).toContain('config.integrations'); - expect(integrationsSkill?.instructions).toContain( - 'If multiple existing integrations match the requested platform, ask which one', - ); - expect(integrationsSkill?.instructions).toContain( - 'Do not call `configure_channel` to remove a channel.', - ); - expect(getConfigMutationPrompt()).toContain('#### Remove An Existing Chat Integration'); - expect(getConfigMutationPrompt()).toContain( - 'Omitting `integrations` preserves existing channels.', - ); - }); -}); - -describe('external integration routing guidance', () => { - it('routes chat and trigger surfaces before resolving callable services', () => { - const toolsPrompt = normalizeWhitespace(TOOLS_PROMPT); - - expect(toolsPrompt).toContain( - "first decide whether it is the target agent's chat or trigger surface", - ); - expect(toolsPrompt).toContain( - 'Do not call `resolve_integration` for chat/trigger integrations.', - ); - expect(toolsPrompt).toContain( - 'For each requested non-chat callable service, call `resolve_integration` separately', - ); - - const slackFlow = normalizeWhitespace( - FEW_SHOT_FLOWS_SECTION.split('### New agent: "Use Anthropic via OpenRouter"')[0], - ); - expect(slackFlow).toContain('`list_integration_types()`'); - expect(slackFlow).toContain('`configure_channel({ integrationType: "slack" })`'); - expect(slackFlow).not.toContain('`resolve_integration`'); - }); - - it('selects an MCP candidate before credential and verification', () => { - const mcpSkill = getBuilderRuntimeSkills().find((skill) => skill.id === 'agent-builder-mcp'); - const mcpInstructions = normalizeWhitespace(mcpSkill?.instructions); - - expect(mcpInstructions).toContain( - '`resolve_integration` returns `{ kind: "mcp", results: [...] }`', - ); - expect(mcpInstructions).toContain('Never read server fields from the wrapper'); - expect(mcpInstructions).toContain('never choose by array order'); - expect(mcpInstructions).toContain('call `ask_questions` with the candidate'); - expect(mcpInstructions).toContain('`selectedResult.credentialType`'); - expect(mcpInstructions).toContain('returned `credentialId` as `credential`'); - expect(mcpInstructions).toContain( - 'If `ask_questions` returns `{ answered: false }`, stop MCP setup', - ); - expect(mcpSkill?.recommendedTools).toContain('ask_questions'); - - const notionFlow = normalizeWhitespace( - FEW_SHOT_FLOWS_SECTION.split('### Add MCP integration:')[1]?.split( - '### Ambiguous request:', - )[0], - ); - expect(notionFlow).toContain('select one entry from `results[]`'); - expect(notionFlow).toContain('`selectedResult.credentialType`'); - expect(notionFlow).toContain('`credentialId` as `credential`'); - }); - - it('stops after channel setup and mutates config only for the non-chat branch', () => { - const ambiguousFlow = normalizeWhitespace( - FEW_SHOT_FLOWS_SECTION.split('### Ambiguous request:')[1]?.split( - '### Publish after build:', - )[0], - ); - - expect(ambiguousFlow).toContain('After `configure_channel` returns, stop this flow'); - expect(ambiguousFlow).toContain('In this non-chat branch only, `read_config()`'); - - const chatBranch = ambiguousFlow.split('If it is a chat integration')[1]?.split('Otherwise')[0]; - expect(chatBranch).not.toContain('`read_config`'); - expect(chatBranch).not.toContain('`patch_config`'); - expect(chatBranch).not.toContain('`write_config`'); - }); -}); - -describe('MCP skill availability', () => { - it('includes the MCP skill', () => { - const skills = getBuilderRuntimeSkills(); - expect(skills.find((s) => s.id === 'agent-builder-mcp')).toBeDefined(); - }); -}); - -describe('resource locator skill availability', () => { - it('includes builder guidance for node dynamic selectors', () => { - const skills = getBuilderRuntimeSkills(); - const skill = skills.find((s) => s.id === 'agent-builder-resource-locators'); - - expect(skill).toBeDefined(); - expect(skill?.description).toContain('write_config/patch_config rejects $fromAI'); - expect(skill?.instructions).toContain('Linear `teamId`'); - expect(skill?.instructions).toContain('get_resource_locator_options'); - expect(skill?.instructions).toContain('parameterValue'); - }); -}); - -describe('sub-agent skill availability', () => { - it('contains the moved sub-agent delegation guidance', () => { - const skill = getBuilderRuntimeSkills().find((s) => s.id === 'agent-builder-sub-agents'); - - expect(skill).toBeDefined(); - expect(skill?.instructions).toContain('`delegate_subagent`'); - expect(skill?.instructions).toContain('Call `list_sub_agents`'); - expect(skill?.instructions).toContain('`type: "multi"`'); - expect(skill?.instructions).toContain('subAgentId: "inline"'); - expect(skill?.instructions).toContain('`subAgents.maxChildren`'); - expect(skill?.instructions).toContain( - '{ "agentId": "", "useWhen": "Use for ..." }', - ); - expect(skill?.instructions).toContain( - 'If it is unclear when a selected saved subagent should be used, ask the user', - ); - expect(skill?.instructions).toContain('Do not write vague values'); - expect(skill?.instructions).toContain( - `from ${SUB_AGENT_MAX_CHILDREN_MIN} to ${SUB_AGENT_MAX_CHILDREN_MAX}`, - ); - }); -}); diff --git a/packages/cli/src/modules/agents/builder/__tests__/agents-builder.service.session.test.ts b/packages/cli/src/modules/agents/builder/__tests__/agents-builder.service.session.test.ts index 839c3248f50..3ad094caccd 100644 --- a/packages/cli/src/modules/agents/builder/__tests__/agents-builder.service.session.test.ts +++ b/packages/cli/src/modules/agents/builder/__tests__/agents-builder.service.session.test.ts @@ -116,6 +116,10 @@ const agentsSdkMocks = vi.hoisted(() => { return { model, options, kind: 'reflect' }; } + function createPlannerTodosTool(): BuiltTool { + return { name: 'write_todos', description: 'planner todos tool' } as BuiltTool; + } + return { streamCalls, instructionsCalls, @@ -131,6 +135,7 @@ const agentsSdkMocks = vi.hoisted(() => { MockMemory, createObservationLogObserveFn, createObservationLogReflectFn, + createPlannerTodosTool, }; }); @@ -139,6 +144,7 @@ vi.mock('@n8n/agents', () => ({ Memory: agentsSdkMocks.MockMemory, createObservationLogObserveFn: agentsSdkMocks.createObservationLogObserveFn, createObservationLogReflectFn: agentsSdkMocks.createObservationLogReflectFn, + createPlannerTodosTool: agentsSdkMocks.createPlannerTodosTool, })); // Avoid a real `models.dev` catalog fetch — irrelevant to thread isolation and @@ -295,7 +301,7 @@ describe('AgentsBuilderService session isolation', () => { expect(n8nCheckpointStorage.delete).toHaveBeenCalledWith('run-1', 'agent-1'); }); - it('includes the integrations skill', async () => { + it('includes the external services skill', async () => { const { service, user, credentialProvider } = setup(); await drain( @@ -303,7 +309,7 @@ describe('AgentsBuilderService session isolation', () => { ); const skills = agentsSdkMocks.skillsCalls[0] as Array<{ id: string }>; - expect(skills.some((skill) => skill.id === 'agent-builder-integrations')).toBe(true); + expect(skills.some((skill) => skill.id === 'agent-builder-external-services')).toBe(true); }); it('uses session.modelConfig directly for the builder model', async () => { diff --git a/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts b/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts index f2653629567..a2971e5c486 100644 --- a/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts +++ b/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts @@ -22,12 +22,13 @@ vi.mock('../../json-config/mcp-client-factory', () => ({ // Helpers // --------------------------------------------------------------------------- -function makeDeps() { +function makeDeps(overrides: Partial[0]> = {}) { return { credentialProvider: mock(), oauthService: mock(), projectId: 'proj-1', proxyFetch: vi.fn() as unknown as CustomFetch, + ...overrides, }; } @@ -256,4 +257,136 @@ describe('buildVerifyMcpServerTool', () => { expect(result).toEqual({ ok: false, error: 'MCP server verification was cancelled' }); expect(closeMock).toHaveBeenCalledTimes(1); }); + + it('auto-applies the credential when verification succeeds and the callback reports applied', async () => { + const applyCredentialToMcpServer = vi.fn().mockResolvedValue({ applied: true }); + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockResolvedValue([{ name: 'echo', description: 'Echo the input' }]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool( + makeDeps({ + agentId: 'agent-1', + applyCredentialToMcpServer, + }), + ); + const result = await tool.handler!( + { + name: 'notion', + url: 'https://example.test/mcp', + credential: 'cred-42', + }, + {} as never, + ); + + expect(applyCredentialToMcpServer).toHaveBeenCalledWith('notion', 'cred-42'); + expect(result).toEqual({ + ok: true, + tools: [{ name: 'echo', description: 'Echo the input' }], + credentialApplied: true, + configMutated: true, + agentId: 'agent-1', + }); + }); + + it('returns a plain success result when the callback reports not applied', async () => { + const applyCredentialToMcpServer = vi.fn().mockResolvedValue({ applied: false }); + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockResolvedValue([{ name: 'echo', description: 'Echo the input' }]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool( + makeDeps({ + agentId: 'agent-1', + applyCredentialToMcpServer, + }), + ); + const result = await tool.handler!( + { + name: 'notion', + url: 'https://example.test/mcp', + credential: 'cred-42', + }, + {} as never, + ); + + expect(result).toEqual({ + ok: true, + tools: [{ name: 'echo', description: 'Echo the input' }], + }); + }); + + it('returns credentialApplied false when the callback throws', async () => { + const applyCredentialToMcpServer = vi.fn().mockRejectedValue(new Error('config write failed')); + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockResolvedValue([{ name: 'echo', description: 'Echo the input' }]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool( + makeDeps({ + agentId: 'agent-1', + applyCredentialToMcpServer, + }), + ); + const result = await tool.handler!( + { + name: 'notion', + url: 'https://example.test/mcp', + credential: 'cred-42', + }, + {} as never, + ); + + expect(result).toEqual({ + ok: true, + tools: [{ name: 'echo', description: 'Echo the input' }], + credentialApplied: false, + }); + }); + + it('does not call the credential callback when verification fails', async () => { + const applyCredentialToMcpServer = vi.fn(); + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockRejectedValue(new Error('connection timeout')), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool( + makeDeps({ + agentId: 'agent-1', + applyCredentialToMcpServer, + }), + ); + await tool.handler!( + { + name: 'notion', + url: 'https://example.test/mcp', + credential: 'cred-42', + }, + {} as never, + ); + + expect(applyCredentialToMcpServer).not.toHaveBeenCalled(); + }); + + it('does not call the credential callback when no credential is provided', async () => { + const applyCredentialToMcpServer = vi.fn(); + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockResolvedValue([{ name: 'echo', description: 'Echo the input' }]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool( + makeDeps({ + agentId: 'agent-1', + applyCredentialToMcpServer, + }), + ); + await tool.handler!({ name: 'notion', url: 'https://example.test/mcp' }, {} as never); + + expect(applyCredentialToMcpServer).not.toHaveBeenCalled(); + }); }); diff --git a/packages/cli/src/modules/agents/builder/agents-builder-model-recommendations.ts b/packages/cli/src/modules/agents/builder/agents-builder-model-recommendations.ts index e37e7b86a72..05b0f7f01da 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-model-recommendations.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-model-recommendations.ts @@ -82,11 +82,13 @@ tool-capable. Treat this list as authoritative for model recommendations. Use these models when the user does not know what model to pick. Prefer a recommended model for a provider the user has credentials for; then call resolve_llm with that -provider and model, or ask via ask_questions if the user needs to choose a -credential. +provider and model. During an initial build, if the user still needs to +choose a provider or credential, do not ask: include the model choice in the +trailing finish_setup call instead. Do not mention models outside this list unless the user explicitly names one -and resolve_llm validates it. Do not write a model or credential directly -without a resolve_llm result. +and resolve_llm validates it. Never write a non-empty model or credential +without a resolve_llm result; drafts carry model "" while LLM setup is +pending. ${rows.join('\n')}`; } diff --git a/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts b/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts index 0d09ae5c3cd..59ff58cec5a 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts @@ -1,4 +1,5 @@ import { getConfigMutationPrompt } from './prompts/config-mutation.prompt'; +import { INITIAL_BUILD_SECTION } from './prompts/initial-build.prompt'; import { getLlmSelectionPrompt } from './prompts/llm-selection.prompt'; import { MEMORY_PROMPT } from './prompts/memory.prompt'; import { TOOLS_PROMPT } from './prompts/tools.prompt'; @@ -26,8 +27,9 @@ agent in this Build chat, do not call tools. Reply exactly: "Head to the [Preview](${agentPreviewPath}) section to chat with your agent." Do not say anything else. Keep the Preview link as a relative app path. -Never write empty, placeholder, or guessed \`instructions\`. If you do not have -enough detail to write meaningful instructions, ask the user first.`; +Never write empty or placeholder \`instructions\`. When the user gave a +concrete goal, write real instructions from it and fill gaps with sensible assumptions +stated in your summary. Only ask first when the overall goal itself is missing.`; } export const INTERACTIVE_TOOLS_SECTION = `\ @@ -47,14 +49,32 @@ Exception: the opening reply to a greeting, a "what do you do", or a vague intent — there you reply conversationally and ask for the overall goal, per "When To Build vs When To Converse". +"Initial build" means the first build pass on a fresh agent; per the Initial +Build section, never suspend during it except the single trailing +\`finish_setup\` call. Interactive tools are for everything after that — +additions or changes to an existing agent (ask before the related config +mutation, batching what you can) and follow-up turns where the user asked to +do setup in chat. + +- \`finish_setup\`: use ONCE, only in the trailing step of an initial build + when only blocked tasks remain — the model choice and every open decision + as \`questions\`, one \`credentialRequests\` entry per credential slot, and + one \`channels\` entry per drafted channel integration. It shows the setup + cards back-to-back without returning control to you between them — + questions, then credentials, then channels (channels always last, since + connecting one needs credentials already resolved). Never call it + together with another interactive tool. - \`ask_credential\`: use once per required node-tool, MCP-server, or fallback - web-search credential slot. For node tools and fallback web search, call it - before the related config mutation; for MCP servers, call it before - verification. NEVER use it for a chat-channel + web-search credential slot. During an initial build, never call it + (see Initial Build). For an addition to an existing + agent, call it before the related config mutation. For MCP servers, call it + before verification. NEVER use it for a chat-channel credential — use \`configure_channel\` instead. - \`configure_channel\`: ALWAYS use this to connect a chat platform (Slack, Telegram, ...) as an agent channel, with a type from \`list_integration_types\`. - The setup UI creates and persists the credential itself. + The setup UI creates and persists the credential itself. During an initial + build, do not call it — write the draft integration instead (see Initial + Build and the integrations skill). - \`ask_questions\`: the default way to ask the user anything that isn't a node-tool credential, MCP-server credential, fallback web-search credential, or channel choice, including when the user must choose, confirm, configure, or @@ -63,8 +83,11 @@ intent — there you reply conversationally and ask for the overall goal, per question you currently need into a single call instead of asking one at a time. Each question is single-select, multi-select, or free-text; pass discrete \`options\` for a known small set of choices, or \`type: "text"\` for - an open-ended question. + an open-ended question. Never call it during an initial build + (see Initial Build). - Never call two interactive tools in parallel. The run suspends on the first. +- Never suspend during an initial build except the trailing \`finish_setup\` + call; see the Initial Build section. - Never re-ask a question the user already answered in this thread. - After resume, continue with the next concrete tool action. Do not narrate the answer back to the user.`; @@ -103,21 +126,37 @@ export const RESPONSE_STYLE_SECTION = `\ Be concise. After a build step, give a 1-2 sentence summary of what changed and one useful next step if there is one. Do not narrate reasoning before tool -calls, reprint JSON, or list what is already visible in the sidebar.`; +calls, reprint JSON, or list what is already visible in the sidebar. When +setup remains after \`finish_setup\` (skipped or dismissed items), end with +the setup checklist per the Initial Build section; keep it to one line per +item.`; export const WORKFLOW_SECTION = `\ ## Workflow -1. Clarify missing decisions through the Interactive tools, batching questions. -2. For fresh agents, resolve the main model and credential with \`resolve_llm\`. -3. Draft real target-agent \`instructions\`; never write empty placeholders. +1. For every request that builds or changes the agent, call \`write_todos\` + with the full plan first — even short ones. Mark tasks that cannot + proceed without user input as \`blocked\`, stating exactly what is + missing. +2. For fresh agents, call \`resolve_llm\` once, silently. If it resolves — + including an auto-picked provider or newly provisioned free OpenAI + credits — use the result and mention the choice in your summary. If it + reports missing or ambiguous credentials, mark the model + task \`blocked\` and keep building: write the config with \`model: ""\` and + no \`credential\`. +3. Draft real target-agent \`instructions\` and write the config early; never + write empty placeholders, and never wait for setup answers before writing + instructions, tools, skills, or tasks. 4. Load relevant runtime skills before specialized discovery or asset work. 5. Perform discovery and create any requested tools, skills, or tasks. 6. Follow Config Freshness immediately before every config mutation. 7. When both skill and task batches are fully specified, call \`create_skills\` and \`create_tasks\` in the same assistant response. Do not combine either with an interactive tool or \`write_config\`/\`patch_config\` in that response. -8. When the user asks to publish, activate, or make the agent live/usable, call +8. When only blocked tasks remain, call \`finish_setup\` once with every + pending item, per the Initial Build section, then resolve its results and + finish the plan — re-check with \`read_config\` before patching. +9. When the user asks to publish, activate, or make the agent live/usable, call \`publish_agent\`. Never tell them to click Publish in the editor. Do not auto-publish without that intent. Use \`unpublish_agent\` when they ask to unpublish.`; @@ -126,29 +165,41 @@ export const FEW_SHOT_FLOWS_SECTION = `\ ## Example flows ### New agent: "Build me an agent teammates can @mention in Slack to triage messages" -1. \`ask_questions({ ... })\` for the model choice, then - \`resolve_llm({ provider, model })\` -> resolved provider, model, and credential. +1. \`write_todos\` with the plan. \`resolve_llm({})\` once, silently; if it + reports missing credentials, mark the model task \`blocked\`. 2. \`read_config()\`. -3. \`write_config(...)\` with the model, credential, and instructions. -4. Load \`agent-builder-integrations\`, call \`list_integration_types()\`, and - select the returned Slack type. -5. \`configure_channel({ integrationType: "slack" })\`. The setup UI persists or - skips the channel; do not follow it with a config read or mutation. +3. \`write_config(...)\` with the instructions, and the resolved model and + credential — or \`model: ""\` and no \`credential\` while the model task + is blocked. +4. Load \`agent-builder-external-services\`, call \`list_integration_types()\`, + \`read_config()\`, then \`patch_config(...)\` adding the returned Slack type + to \`/integrations/-\` with \`credentialId: ""\`. +5. \`finish_setup({ channels: [{ integrationType: "slack" }] })\` — include + \`questions: []\` only if the model task is blocked; when + \`resolve_llm\` already resolved in step 1, pass only the channel. For a + model answer, call \`resolve_llm\` with it, then \`read_config()\` and + \`patch_config(...)\` replacing \`/model\` and \`/credential\`. The channel + card in \`finish_setup\` already persisted or skipped the Slack + connection — do not follow it with a config mutation. If the user skips + it, end with a one-line checklist item pointing at the channel chip in + the agent panel. ### New agent: "Use Anthropic via OpenRouter" -1. \`resolve_llm({ provider: "openrouter" })\`. -2. \`read_config()\`. -3. \`write_config(...)\` with \`model: "openrouter/{resolvedModel}"\`, +1. \`write_todos\` with the plan. +2. \`resolve_llm({ provider: "openrouter" })\`. +3. \`read_config()\`. +4. \`write_config(...)\` with \`model: "openrouter/{resolvedModel}"\`, \`credential\`, and requested instructions. ### Change the existing model -1. \`ask_questions({ ... })\` for the new model choice, then +1. \`write_todos\` with the plan. +2. \`ask_questions({ ... })\` for the new model choice, then \`resolve_llm({ provider, model })\`. -2. \`read_config()\`. -3. \`patch_config(...)\` replacing \`/model\` and \`/credential\`. +3. \`read_config()\`. +4. \`patch_config(...)\` replacing \`/model\` and \`/credential\`. ### Add an explicitly requested n8n node tool to an existing agent -1. Load \`agent-builder-node-tools\`, then call \`search_nodes\` and +1. Load \`agent-builder-external-services\`, then call \`search_nodes\` and \`get_node_types\`; the explicit n8n-node request does not need \`resolve_integration\`. 2. \`ask_credential\` for every required slot. @@ -156,16 +207,27 @@ export const FEW_SHOT_FLOWS_SECTION = `\ 4. \`patch_config(...)\` adding the node tool to \`/tools/-\`. ### Add an explicitly requested n8n node tool when credential setup is skipped -1. Load \`agent-builder-node-tools\`, then call \`search_nodes\` and +1. Load \`agent-builder-external-services\`, then call \`search_nodes\` and \`get_node_types\`. 2. \`ask_credential(...)\` -> \`{ skipped: true }\`. 3. \`read_config()\`. 4. \`patch_config(...)\` adding the tool and omitting only the skipped credential slot. Do not abort the tool addition. +5. Summarize it as a successful addition, not a failure: the tool is in + place and starts working once a credential is connected. Never say you + could not complete it — end with a one-line checklist item for + connecting the credential later. ### Add MCP integration: "Connect Notion MCP" +This flow is user-initiated on an existing agent, so the credential ask is +immediate. During an initial build, pick the best candidate as a stated +assumption, write the draft \`/mcpServers/-\` entry with \`credential\` omitted, +skip verification, and include the credential in the trailing \`finish_setup\` +call; verify with the returned credential id — on success the tool writes the +credential into the matching entry itself; no \`read_config\`/\`patch_config\` +follow-up for the credential. 1. \`resolve_integration({ queries: ["notion"] })\`. -2. When it returns \`kind: "mcp"\`, load \`agent-builder-mcp\`. +2. When it returns \`kind: "mcp"\`, load \`agent-builder-external-services\`. 3. For MCP candidates, select one entry from \`results[]\`. If multiple candidates remain, use \`ask_questions\` with their titles and descriptions; never choose by array order. If the user dismisses the @@ -182,17 +244,18 @@ export const FEW_SHOT_FLOWS_SECTION = `\ ### Ambiguous request: "Make it post somewhere" 1. \`ask_questions(...)\` with the known destination choices. -2. Load \`agent-builder-integrations\` to decide whether the destination is the - agent's chat/trigger surface. +2. Load \`agent-builder-external-services\` to decide whether the destination is + the agent's chat/trigger surface. 3. If it is a chat integration, call \`configure_channel\` with the returned \`integrationType\`. After \`configure_channel\` returns, stop this flow; the setup UI already persisted or skipped the channel, so do not read or mutate the config. 4. Otherwise call \`resolve_integration({ queries: [""] })\` and follow the returned kind: - - \`kind: "mcp"\`: load \`agent-builder-mcp\`, verify, and wire the MCP server. - - \`kind: "node"\`: load \`agent-builder-node-tools\`, use the returned node - results with \`get_node_types\`, and ask for every required credential. + - \`kind: "mcp"\`: follow the skill's MCP Servers section — verify and wire + the MCP server. + - \`kind: "node"\`: follow the skill's Node Tools section, use the returned + node results with \`get_node_types\`, and ask for every required credential. 5. In this non-chat branch only, \`read_config()\`, then \`patch_config(...)\` or \`write_config(...)\` with the resolved capability. @@ -218,6 +281,7 @@ export function buildBuilderPrompt(ctx: BuilderPromptContext): string { MEMORY_PROMPT, TOOLS_PROMPT, INTERACTIVE_TOOLS_SECTION, + INITIAL_BUILD_SECTION, READ_CONFIG_FRESHNESS_SECTION, WORKFLOW_SECTION, FEW_SHOT_FLOWS_SECTION, 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 d759698c46b..fa814f6e0c2 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 @@ -14,6 +14,7 @@ import { formatZodErrors, PROVIDER_CAPABILITIES, resolvePromptCaching, + AgentJsonConfigSchema, RunnableAgentJsonConfigSchema, sanitizeAgentJsonConfig, tryParseConfigJson, @@ -36,6 +37,8 @@ import { OauthService } from '@/oauth/oauth.service'; import { userHasScopes } from '@/permissions.ee/check-access'; import { AiService } from '@/services/ai.service'; import { DynamicNodeParametersService } from '@/services/dynamic-node-parameters.service'; +import { FreeAiCreditsService } from '@/services/free-ai-credits.service'; +import { Telemetry } from '@/telemetry'; import { createAiMcpFetch } from '@/utils/ai-proxy-fetch'; import { AgentConfigService } from '../agent-config.service'; @@ -44,6 +47,7 @@ import { AgentIntegrationPersistenceService } from '../agent-integration-persist import { AgentPublishService } from '../agent-publish.service'; import { AgentSkillsService } from '../agent-skills.service'; import { AgentTaskService } from '../agent-task.service'; +import { AgentValidationService } from '../agent-validation.service'; import { AgentsToolsService } from '../agents-tools.service'; import { AgentsService } from '../agents.service'; import { AttachableWorkflowsService } from '../attachable-workflows.service'; @@ -55,6 +59,7 @@ import { buildAskEmbeddingCredentialTool, buildAskQuestionsTool, buildConfigureChannelTool, + buildFinishSetupTool, buildResolveLlmTool, } from './interactive'; import type { ModelLookup } from './interactive/resolve-llm.tool'; @@ -131,6 +136,17 @@ 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. + */ +function parseBuilderWriteConfig(incoming: unknown, currentConfig: AgentJsonConfig | null) { + const schema = currentConfig?.model ? RunnableAgentJsonConfigSchema : AgentJsonConfigSchema; + return schema.safeParse(sanitizeAgentJsonConfig(incoming)); +} + /** * Prompt caching is mandatory for OpenAI/Anthropic: this write-path * normalizer guarantees `config.promptCaching` is force-enabled for those @@ -190,8 +206,37 @@ export class AgentsBuilderToolsService { private readonly nodeTypes: NodeTypes, private readonly ssrfConfig: SsrfProtectionConfig, private readonly ssrfProtectionService: SsrfProtectionService, + private readonly freeAiCreditsService: FreeAiCreditsService, + private readonly telemetry: Telemetry, + private readonly agentValidationService: AgentValidationService, ) {} + /** + * Stamps `configMutated: true` + the target agentId onto successful results of + * config-mutating tools, so the FE can refresh the agent artifact panel from a + * single semantic field instead of a per-tool allowlist. + */ + private withConfigMutationMarker(tool: BuiltTool, agentId: string): BuiltTool { + const handler = tool.handler; + if (!handler) return tool; + return { + ...tool, + handler: async (input, ctx) => { + const result = await handler(input, ctx); + if ( + typeof result === 'object' && + result !== null && + (('ok' in result && result.ok === true) || + ('connected' in result && result.connected === true) || + ('completed' in result && result.completed === true)) + ) { + return { ...result, configMutated: true, agentId }; + } + return result; + }, + }; + } + getTools( agentId: string, projectId: string, @@ -235,7 +280,7 @@ export class AgentsBuilderToolsService { 'Create or replace the agent configuration by writing a complete JSON string. ' + 'Requires baseConfigHash from the immediately preceding read_config result — never from a prior ' + 'write_config/patch_config success or from a stale response. ' + - 'Returns { ok: true } on success — no config, hash, or timestamps are returned; call ' + + 'Returns { ok: true, configMutated: true, agentId } on success — no config, hash, or timestamps are returned; call ' + 'read_config again before any later inspection or mutation — or ' + '{ ok: false, stage, errors } with path, message, expected, received fields on failure. ' + 'On stage: "stale", call read_config and retry once using its fresh config and configHash.', @@ -270,9 +315,7 @@ export class AgentsBuilderToolsService { if (baseConfigHash !== snapshot.configHash) { return { ok: false, stage: 'stale', errors: [STALE_CONFIG_ERROR] }; } - const zodResult = RunnableAgentJsonConfigSchema.safeParse( - sanitizeAgentJsonConfig(parsed.data), - ); + const zodResult = parseBuilderWriteConfig(parsed.data, snapshot.config); if (!zodResult.success) { return { ok: false, errors: formatZodErrors(zodResult.error) }; } @@ -323,7 +366,7 @@ export class AgentsBuilderToolsService { 'Requires baseConfigHash from the immediately preceding read_config result — never from a prior ' + 'write_config/patch_config success or from a stale response. ' + 'Supported ops: add, remove, replace, move, copy, test. ' + - 'Returns { ok: true } on success — no config, hash, or timestamps are returned; call ' + + 'Returns { ok: true, configMutated: true, agentId } on success — no config, hash, or timestamps are returned; call ' + 'read_config again before any later inspection or mutation — or ' + '{ ok: false, stage, errors } on failure. ' + 'stage is "parse", "stale", "patch", or "schema". On stage: "stale", call read_config and retry ' + @@ -390,9 +433,7 @@ export class AgentsBuilderToolsService { const patched = jsonpatch.applyPatch(jsonpatch.deepClone(snapshot.config), ops) .newDocument as unknown as AgentJsonConfig; - const zodResult = RunnableAgentJsonConfigSchema.safeParse( - sanitizeAgentJsonConfig(patched), - ); + const zodResult = parseBuilderWriteConfig(patched, snapshot.config); if (!zodResult.success) { return { ok: false, stage: 'schema', errors: formatZodErrors(zodResult.error) }; } @@ -475,7 +516,7 @@ export class AgentsBuilderToolsService { 'Idempotent when the draft is already the active published version. Pass optional `versionId` to ' + 'activate an existing history row instead of publishing the current draft. Call only when the user ' + 'asks to publish, activate, or make the agent live/usable — never tell them to click Publish in the editor. ' + - 'Returns { ok: true, agentId, activeVersionId, versionId } or { ok: false, errors }.', + 'Returns { ok: true, configMutated: true, agentId, activeVersionId, versionId } or { ok: false, errors }.', ) .input( z.object({ @@ -521,7 +562,7 @@ export class AgentsBuilderToolsService { .description( 'Unpublish this target agent: clears the live version while preserving the draft, disconnects chat ' + 'integrations, and stops scheduled tasks. Call when the user asks to unpublish or take the agent offline. ' + - 'Returns { ok: true, agentId, activeVersionId: null } or { ok: false, errors }.', + 'Returns { ok: true, configMutated: true, agentId, activeVersionId: null } or { ok: false, errors }.', ) .input(z.object({})) .handler(async () => { @@ -558,13 +599,27 @@ export class AgentsBuilderToolsService { const tools: BuiltTool[] = [ readConfigTool, - writeConfigTool, - patchConfigTool, + this.withConfigMutationMarker(writeConfigTool, agentId), + this.withConfigMutationMarker(patchConfigTool, agentId), listIntegrationTypesTool, listSubAgentsTool, - publishAgentTool, - unpublishAgentTool, - buildResolveLlmTool({ credentialProvider, modelLookup }), + this.withConfigMutationMarker(publishAgentTool, agentId), + this.withConfigMutationMarker(unpublishAgentTool, agentId), + buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: { + isEligible: () => this.freeAiCreditsService.isEligible(user), + claim: async () => { + const credential = await this.freeAiCreditsService.claim(user, projectId); + this.telemetry.track('User claimed OpenAI credits', { + user_id: user.id, + source: 'agentBuilderResolveLlm', + }); + return { credentialId: credential.id, credentialName: credential.name }; + }, + }, + }), buildAskCredentialTool({ credentialProvider, isCredentialTypeKnown: (credentialType) => this.credentialTypes.recognizes(credentialType), @@ -581,15 +636,55 @@ export class AgentsBuilderToolsService { isAssistantProxyEnabled: () => this.aiService.isProxyEnabled(), }), buildAskQuestionsTool(), - buildConfigureChannelTool({ + this.withConfigMutationMarker( + buildConfigureChannelTool({ + agentId, + projectId, + listChatIntegrationTypes: () => + this.agentIntegrationPersistenceService + .listChatIntegrations() + .map((integration) => integration.type), + }), agentId, - projectId, - listChatIntegrationTypes: () => - this.agentIntegrationPersistenceService - .listChatIntegrations() - .map((integration) => integration.type), - }), + ), + this.withConfigMutationMarker( + buildFinishSetupTool({ + credentialProvider, + agentId, + projectId, + isCredentialTypeKnown: (credentialType) => + this.credentialTypes.recognizes(credentialType), + listIntegrationCredentialIds: async () => { + const agent = await this.agentsService.findById(agentId, projectId); + return (agent?.integrations ?? []) + .map((integration) => integration.credentialId) + .filter((credentialId) => credentialId.length > 0); + }, + listChatIntegrationTypes: () => + this.agentIntegrationPersistenceService + .listChatIntegrations() + .map((integration) => integration.type), + getPublishBlockers: async () => { + // Connecting a channel auto-publishes the agent, so gate it on the + // same publish validation. Integration issues are excluded: the + // draft channel entry itself (`credentialId: ""`) always reports + // missing_credential, and that's exactly what this channel phase + // is about to resolve. + const { issues } = await this.agentValidationService.validateAgentConfiguration( + agentId, + projectId, + credentialProvider, + 'publish', + ); + return issues + .filter((issue) => !issue.path.startsWith('integrations.')) + .map((issue) => ({ path: issue.path, code: issue.code })); + }, + }), + agentId, + ), buildVerifyMcpServerTool({ + agentId, credentialProvider, oauthService: this.oauthService, projectId, @@ -598,6 +693,8 @@ export class AgentsBuilderToolsService { this.ssrfConfig, this.ssrfProtectionService, ), + applyCredentialToMcpServer: async (serverName, credentialId) => + await this.applyCredentialToMcpServer(agentId, projectId, serverName, credentialId), }), buildSearchMcpServersTool({ mcpRegistryService: this.mcpRegistryService }), buildResolveIntegrationTool({ @@ -668,8 +765,9 @@ export class AgentsBuilderToolsService { 'runtime uses to decide when to load the skill; the instructions must follow the required ' + 'structured Markdown template (Overview, Inputs, Steps, Rules, Example, Gotchas) with each ' + 'applicable section filled in with concrete, specific content. If you do not have enough domain ' + - 'detail to write a genuinely useful skill, ask the user clarifying questions until you do before ' + - 'calling create_skills. Use allowedTools only with exact target-agent tool names. Use references ' + + "detail to write a genuinely useful skill, derive it from the user's goal as stated assumptions " + + 'listed in your summary; ask the user clarifying questions only when even a reasonable ' + + 'assumption is impossible. Use allowedTools only with exact target-agent tool names. Use references ' + 'only for markdown supporting files under the references/ directory — references are not ' + 'automatically loaded, so instructions must say exactly when to load each one by path; scripts and ' + 'non-markdown linked files are not supported. Do not invent tool names or reference paths. Batch ' + @@ -710,7 +808,7 @@ export class AgentsBuilderToolsService { 'objective field carries its own structured template. The whole batch is all-or-nothing: an ' + 'invalid cron or objective rejects every task in the call. This adds a `{ type: "task", id, ' + 'enabled }` ref per task to the agent config (config.tasks) and each task starts running once ' + - 'the agent is (re)published via `publish_agent`. Returns { ok: true, tasks: [{ id, name, enabled }, ...] } (same ' + + 'the agent is (re)published via `publish_agent`. Returns { ok: true, configMutated: true, agentId, tasks: [{ id, name, enabled }, ...] } (same ' + 'order as input, objectives and crons are not echoed back) or { ok: false, errors }.', ) .systemInstruction( @@ -718,8 +816,9 @@ export class AgentsBuilderToolsService { 'required section, or an unclear schedule. Each objective must follow the required structured ' + 'Markdown template (Objective, Context, Steps, Output, Constraints, Success criteria) with every ' + 'section filled in with concrete content — it is the exact, self-contained message the agent ' + - 'receives on each unattended run. If anything is ambiguous, ask the user clarifying questions ' + - '(ask_questions with discrete options for choices, or type: "text" for open-ended) before calling ' + + "receives on each unattended run. If anything is ambiguous, derive it from the user's goal as " + + 'stated assumptions listed in your summary; ask the user clarifying questions with ask_questions ' + + 'only when even a reasonable assumption is impossible, before calling ' + 'create_tasks. A task can only use tools the agent already has: if any step in an objective ' + 'requires a tool, integration, or web search the agent is missing, you MUST add it to the agent ' + 'config (patch_config/write_config) BEFORE calling create_tasks — otherwise the task will fail at ' + @@ -797,7 +896,7 @@ export class AgentsBuilderToolsService { return [ buildCustomToolTool, createSkillsTool, - createTasksTool, + this.withConfigMutationMarker(createTasksTool, agentId), listWorkflowsTool, buildGetResourceLocatorOptionsTool({ dynamicNodeParametersService: this.dynamicNodeParametersService, @@ -824,4 +923,47 @@ export class AgentsBuilderToolsService { const config = composeJsonConfig(agent); return snapshotFromConfig(config); } + + private async applyCredentialToMcpServer( + agentId: string, + projectId: string, + serverName: string, + credentialId: string, + ): Promise<{ applied: boolean }> { + const snapshot = await this.getConfigSnapshot(agentId, projectId); + const config = snapshot.config; + const servers = config?.mcpServers; + if (!config || !servers) { + return { applied: false }; + } + + const serverIndex = servers.findIndex((server) => server.name === serverName); + if (serverIndex === -1) { + return { applied: false }; + } + + if (servers[serverIndex]?.credential === credentialId) { + return { applied: false }; + } + + // Only one field changes, so shallow copies are enough — no deep clone. + const patched: AgentJsonConfig = { + ...config, + mcpServers: servers.map((server, index) => + index === serverIndex ? { ...server, credential: credentialId } : server, + ), + }; + + const zodResult = parseBuilderWriteConfig(patched, snapshot.config); + if (!zodResult.success) { + throw new Error(formatZodErrors(zodResult.error)[0]?.message ?? 'Invalid MCP server config'); + } + + const configWithDefaults = applyPromptCachingBuilderDefaults( + applyNativeWebSearchDefaultOn(zodResult.data), + ); + + await this.agentConfigService.updateConfig(agentId, projectId, configWithDefaults); + return { applied: true }; + } } diff --git a/packages/cli/src/modules/agents/builder/agents-builder.service.ts b/packages/cli/src/modules/agents/builder/agents-builder.service.ts index fc600cb2229..0d1013f7113 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder.service.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder.service.ts @@ -28,6 +28,10 @@ import { getModelRecommendationsSection } from './agents-builder-model-recommend import { buildBuilderPrompt } from './agents-builder-prompts'; import { AgentsBuilderToolsService } from './agents-builder-tools.service'; import { BuilderCheckpointUnavailableError } from './errors'; +import { + BUILDER_PLANNER_TODOS_DESCRIPTION, + BUILDER_PLANNER_TODOS_SYSTEM_INSTRUCTION, +} from './prompts/planner-todos.prompt'; import { getBuilderRuntimeSkills } from './skills'; import { N8NCheckpointStorage } from '../integrations/n8n-checkpoint-storage'; import { N8nMemory } from '../integrations/n8n-memory'; @@ -229,7 +233,7 @@ export class AgentsBuilderService { user, ); - const { Agent, Memory } = await import('@n8n/agents'); + const { Agent, Memory, createPlannerTodosTool } = await import('@n8n/agents'); const onMemoryUsage = async (report: MemoryTaskUsageReport) => { try { @@ -276,6 +280,13 @@ export class AgentsBuilderService { builder.tool(tool); } + builder.tool( + createPlannerTodosTool({ + description: BUILDER_PLANNER_TODOS_DESCRIPTION, + systemInstruction: BUILDER_PLANNER_TODOS_SYSTEM_INSTRUCTION, + }), + ); + applyAgentThinking(builder, modelConfig); return builder; diff --git a/packages/cli/src/modules/agents/builder/builder-tool-names.ts b/packages/cli/src/modules/agents/builder/builder-tool-names.ts index 6a8fe33d955..eb7243c8bec 100644 --- a/packages/cli/src/modules/agents/builder/builder-tool-names.ts +++ b/packages/cli/src/modules/agents/builder/builder-tool-names.ts @@ -17,6 +17,7 @@ export const BUILDER_TOOLS = { BUILD_CUSTOM_TOOL: 'build_custom_tool', CREATE_SKILLS: 'create_skills', CREATE_TASKS: 'create_tasks', + FINISH_SETUP: 'finish_setup', GET_RESOURCE_LOCATOR_OPTIONS: 'get_resource_locator_options', LIST_INTEGRATION_TYPES: 'list_integration_types', LIST_SUB_AGENTS: 'list_sub_agents', diff --git a/packages/cli/src/modules/agents/builder/interactive/__tests__/finish-setup.tool.test.ts b/packages/cli/src/modules/agents/builder/interactive/__tests__/finish-setup.tool.test.ts new file mode 100644 index 00000000000..9a6ee2ccc57 --- /dev/null +++ b/packages/cli/src/modules/agents/builder/interactive/__tests__/finish-setup.tool.test.ts @@ -0,0 +1,353 @@ +import type { CredentialListItem, CredentialProvider } from '@n8n/agents'; +import type { Mock } from 'vitest'; +import type { z } from 'zod'; + +import { buildFinishSetupTool } from '../finish-setup.tool'; + +interface TestCtx { + resumeData?: unknown; + suspendPayload?: unknown; + suspend: Mock; +} + +function makeCtx(overrides?: { resumeData?: unknown; suspendPayload?: unknown }): TestCtx { + return { + resumeData: overrides?.resumeData, + suspendPayload: overrides?.suspendPayload, + suspend: vi.fn(async (payload: unknown) => payload), + }; +} + +function makeProvider(creds: CredentialListItem[]): CredentialProvider { + return { + list: vi.fn(async () => creds), + resolve: vi.fn(async () => ({})), + }; +} + +const BASE_DEPS = { + agentId: 'agent-1', + projectId: 'project-1', + listChatIntegrationTypes: () => ['slack', 'telegram'], + getPublishBlockers: async () => [], +}; + +describe('finish_setup tool', () => { + it('auto-resolves single-credential and channel-matching slots, excluding them from the credential phase', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'My Airtable', type: 'airtableApi' }, + { id: 'c2', name: 'Personal Slack', type: 'slackApi' }, + { id: 'c3', name: 'Notion A', type: 'notionApi' }, + { id: 'c4', name: 'Notion B', type: 'notionApi' }, + ]); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider, + listIntegrationCredentialIds: async () => ['c2'], + }); + const ctx = makeCtx(); + + const payload = (await tool.handler!( + { + credentialRequests: [ + { credentialType: 'airtableApi', purpose: 'Airtable log' }, + { credentialType: 'slackApi', purpose: 'Slack tool' }, + { credentialType: 'notionApi', purpose: 'Notion search' }, + ], + }, + ctx as never, + )) as Record; + + expect(payload.credentialRequests).toEqual([ + { + credentialType: 'notionApi', + reason: 'Notion search', + existingCredentials: [ + { id: 'c3', name: 'Notion A' }, + { id: 'c4', name: 'Notion B' }, + ], + }, + ]); + expect( + (payload.finishSetupChain as { collected: { credentials: unknown } }).collected.credentials, + ).toEqual({ + airtableApi: { id: 'c1', name: 'My Airtable' }, + slackApi: { id: 'c2', name: 'Personal Slack' }, + }); + }); + + it('returns completed without suspending when every credential slot auto-resolves and there is nothing else pending', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'My Airtable', type: 'airtableApi' }, + ]); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider, + }); + const ctx = makeCtx(); + + const result = await tool.handler!( + { credentialRequests: [{ credentialType: 'airtableApi', purpose: 'Airtable log' }] }, + ctx as never, + ); + + expect(ctx.suspend).not.toHaveBeenCalled(); + expect(result).toEqual({ + completed: true, + credentials: { airtableApi: { id: 'c1', name: 'My Airtable' } }, + }); + }); + + it('chains through questions and credentials to a merged result', async () => { + const credentialProvider = makeProvider([]); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider, + }); + const input = { + questions: [ + { id: 'model', question: 'Which model?', type: 'single' as const, options: ['gpt'] }, + ], + credentialRequests: [{ credentialType: 'airtableApi', purpose: 'Airtable log' }], + }; + + const questionsPayload = (await tool.handler!(input, makeCtx() as never)) as Record< + string, + unknown + >; + expect(questionsPayload).toMatchObject({ + inputType: 'questions', + message: 'Finish setup (1/2)', + questions: [{ id: 'model', question: 'Which model?', type: 'single', options: ['gpt'] }], + }); + expect(questionsPayload.finishSetupChain).toMatchObject({ + currentPhase: { kind: 'questions' }, + remainingPhases: [{ kind: 'credentials' }], + totalPhases: 2, + }); + + const credentialsPayload = (await tool.handler!( + input, + makeCtx({ + resumeData: { + approved: true, + answers: [{ questionId: 'model', selectedOptions: ['gpt'] }], + }, + suspendPayload: questionsPayload, + }) as never, + )) as Record; + expect(credentialsPayload.credentialRequests).toEqual([ + { credentialType: 'airtableApi', reason: 'Airtable log', existingCredentials: [] }, + ]); + expect(credentialsPayload.message).toBe('Finish setup (2/2)'); + + const result = await tool.handler!( + input, + makeCtx({ + resumeData: { credentials: { airtableApi: 'new-cred' } }, + suspendPayload: credentialsPayload, + }) as never, + ); + expect(result).toEqual({ + completed: true, + answers: [{ questionId: 'model', selectedOptions: ['gpt'] }], + credentials: { airtableApi: { id: 'new-cred', name: 'new-cred' } }, + }); + }); + + it('marks the credential slot skipped when the credential phase is skipped', async () => { + const credentialProvider = makeProvider([]); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider, + }); + const input = { + credentialRequests: [{ credentialType: 'airtableApi', purpose: 'Airtable log' }], + }; + + const credentialsPayload = (await tool.handler!(input, makeCtx() as never)) as Record< + string, + unknown + >; + + const result = await tool.handler!( + input, + makeCtx({ resumeData: { skipped: true }, suspendPayload: credentialsPayload }) as never, + ); + expect(result).toEqual({ + completed: true, + credentials: { airtableApi: 'skipped' }, + }); + }); + + it('throws for an unknown credential type', async () => { + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + isCredentialTypeKnown: (credentialType) => credentialType === 'airtableApi', + }); + const ctx = makeCtx(); + + await expect( + tool.handler!( + { credentialRequests: [{ credentialType: 'unknownApi', purpose: 'x' }] }, + ctx as never, + ), + ).rejects.toThrow('Unknown credential type "unknownApi"'); + expect(ctx.suspend).not.toHaveBeenCalled(); + }); + + it('rejects an input with no pending setup items', () => { + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + }); + + expect((tool.inputSchema as unknown as z.ZodTypeAny).safeParse({}).success).toBe(false); + }); + + it('chains through questions, credentials, and multiple channels to a merged result', async () => { + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + }); + const input = { + questions: [ + { id: 'model', question: 'Which model?', type: 'single' as const, options: ['gpt'] }, + ], + credentialRequests: [{ credentialType: 'airtableApi', purpose: 'Airtable log' }], + channels: [{ integrationType: 'slack' }, { integrationType: 'telegram' }], + }; + + const questionsPayload = (await tool.handler!(input, makeCtx() as never)) as Record< + string, + unknown + >; + expect(questionsPayload.message).toBe('Finish setup (1/4)'); + + const credentialsPayload = (await tool.handler!( + input, + makeCtx({ + resumeData: { answers: [{ questionId: 'model', selectedOptions: ['gpt'] }] }, + suspendPayload: questionsPayload, + }) as never, + )) as Record; + expect(credentialsPayload.message).toBe('Finish setup (2/4)'); + + const slackPayload = (await tool.handler!( + input, + makeCtx({ + resumeData: { credentials: { airtableApi: 'new-cred' } }, + suspendPayload: credentialsPayload, + }) as never, + )) as Record; + expect(slackPayload).toMatchObject({ + message: 'Set up the slack channel', + channelConfig: { integrationType: 'slack', agentId: 'agent-1' }, + projectId: 'project-1', + }); + + const telegramPayload = (await tool.handler!( + input, + makeCtx({ resumeData: { approved: false }, suspendPayload: slackPayload }) as never, + )) as Record; + expect(telegramPayload.message).toBe('Set up the telegram channel'); + + const result = await tool.handler!( + input, + makeCtx({ resumeData: { approved: true }, suspendPayload: telegramPayload }) as never, + ); + expect(result).toEqual({ + completed: true, + answers: [{ questionId: 'model', selectedOptions: ['gpt'] }], + credentials: { airtableApi: { id: 'new-cred', name: 'new-cred' } }, + channels: { slack: 'skipped', telegram: 'connected' }, + }); + }); + + it('throws for an unsupported channel type', async () => { + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + }); + const ctx = makeCtx(); + + await expect( + tool.handler!({ channels: [{ integrationType: 'discord' }] }, ctx as never), + ).rejects.toThrow('Unsupported chat channel "discord"'); + expect(ctx.suspend).not.toHaveBeenCalled(); + }); + + it('suspends normally for a channel phase when there are no publish blockers', async () => { + const getPublishBlockers = vi.fn(async () => []); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + getPublishBlockers, + }); + const ctx = makeCtx(); + + const payload = (await tool.handler!( + { channels: [{ integrationType: 'slack' }] }, + ctx as never, + )) as Record; + + expect(getPublishBlockers).toHaveBeenCalled(); + expect(payload).toMatchObject({ + message: 'Set up the slack channel', + channelConfig: { integrationType: 'slack', agentId: 'agent-1' }, + }); + }); + + it('returns the channel blocked without suspending when the agent cannot be published (channel-first)', async () => { + const getPublishBlockers = vi.fn(async () => [{ path: 'model', code: 'missing_required' }]); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + getPublishBlockers, + }); + const ctx = makeCtx(); + + const result = await tool.handler!({ channels: [{ integrationType: 'slack' }] }, ctx as never); + + expect(getPublishBlockers).toHaveBeenCalled(); + expect(ctx.suspend).not.toHaveBeenCalled(); + expect(result).toEqual({ + completed: true, + channels: { slack: 'blocked' }, + publishBlockedIssues: [{ path: 'model', code: 'missing_required' }], + }); + }); + + it('marks every remaining channel phase blocked when publish blockers appear at the first channel entry', async () => { + const getPublishBlockers = vi.fn(async () => [{ path: 'model', code: 'missing_required' }]); + const tool = buildFinishSetupTool({ + ...BASE_DEPS, + credentialProvider: makeProvider([]), + getPublishBlockers, + }); + const input = { + credentialRequests: [{ credentialType: 'airtableApi', purpose: 'Airtable log' }], + channels: [{ integrationType: 'slack' }, { integrationType: 'telegram' }], + }; + + const credentialsPayload = (await tool.handler!(input, makeCtx() as never)) as Record< + string, + unknown + >; + + const resumeCtx = makeCtx({ + resumeData: { credentials: { airtableApi: 'new-cred' } }, + suspendPayload: credentialsPayload, + }); + const result = await tool.handler!(input, resumeCtx as never); + + expect(resumeCtx.suspend).not.toHaveBeenCalled(); + expect(result).toEqual({ + completed: true, + credentials: { airtableApi: { id: 'new-cred', name: 'new-cred' } }, + channels: { slack: 'blocked', telegram: 'blocked' }, + publishBlockedIssues: [{ path: 'model', code: 'missing_required' }], + }); + }); +}); 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 8956e5d7b2c..27feb815ba5 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,8 @@ import type { CredentialListItem, CredentialProvider } from '@n8n/agents'; import type { Mock } from 'vitest'; -import type { ModelLookup } from '../resolve-llm.tool'; +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'; function makeProvider(creds: CredentialListItem[]): CredentialProvider { @@ -17,6 +18,21 @@ function makeModelLookup(impl?: ModelLookup['list']): ModelLookup & { list: Mock }; } +function makeFreeCredits( + isEligibleImpl?: FreeCreditsProvisioner['isEligible'], + claimImpl?: FreeCreditsProvisioner['claim'], +): FreeCreditsProvisioner & { isEligible: Mock; claim: Mock } { + return { + isEligible: vi.fn(isEligibleImpl ?? (() => false)), + claim: vi.fn( + claimImpl ?? + (async () => { + throw new Error('makeFreeCredits: claim() called without an implementation'); + }), + ), + }; +} + describe('resolve_llm tool', () => { it('auto-resolves when exactly one LLM-provider credential exists', async () => { const credentialProvider = makeProvider([ @@ -24,7 +40,11 @@ describe('resolve_llm tool', () => { { id: 'c2', name: 'My Slack', type: 'slackApi' }, ]); const modelLookup = makeModelLookup(); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({}, {}); expect(result).toEqual({ @@ -43,7 +63,11 @@ describe('resolve_llm tool', () => { { id: 'c2', name: 'My OpenRouter', type: 'openRouterApi' }, ]); const modelLookup = makeModelLookup(); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'openrouter' }, {}); expect(result).toEqual({ @@ -61,7 +85,11 @@ describe('resolve_llm tool', () => { { name: 'Grok 4 Fast', value: 'grok-4-fast' }, { name: 'Grok 4', value: 'grok-4' }, ]); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'xai', model: 'grok-4-fast' }, {}); expect(result).toEqual({ @@ -78,7 +106,11 @@ describe('resolve_llm tool', () => { { id: 'c1', name: 'My Anthropic', type: 'anthropicApi' }, ]); const modelLookup = makeModelLookup(); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'openrouter' }, {}); expect(result).toEqual({ @@ -96,7 +128,11 @@ describe('resolve_llm tool', () => { { id: 'c2', name: 'Work OpenRouter', type: 'openRouterApi' }, ]); const modelLookup = makeModelLookup(); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'openrouter' }, {}); expect(result).toEqual({ @@ -111,14 +147,43 @@ describe('resolve_llm tool', () => { }); }); - it('returns ambiguous_provider_or_credential when no provider is requested and multiple credentials exist', async () => { + it('returns ambiguous_provider_or_credential when the top-priority provider has multiple credentials', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'Personal Anthropic', type: 'anthropicApi' }, + { id: 'c2', name: 'Work Anthropic', type: 'anthropicApi' }, + { id: 'c3', name: 'My OpenAI', type: 'openAiApi' }, + ]); + const modelLookup = makeModelLookup(); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); + const result = await tool.handler!({}, {}); + + expect(result).toEqual({ + ok: false, + reason: 'ambiguous_provider_or_credential', + credentials: [ + { id: 'c1', name: 'Personal Anthropic', type: 'anthropicApi', provider: 'anthropic' }, + { id: 'c2', name: 'Work Anthropic', type: 'anthropicApi', provider: 'anthropic' }, + { id: 'c3', name: 'My OpenAI', type: 'openAiApi', provider: 'openai' }, + ], + }); + }); + + it('does not auto-pick when a model is requested without a provider', async () => { const credentialProvider = makeProvider([ { id: 'c1', name: 'My Anthropic', type: 'anthropicApi' }, { id: 'c2', name: 'My OpenAI', type: 'openAiApi' }, ]); const modelLookup = makeModelLookup(); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); - const result = await tool.handler!({}, {}); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); + const result = await tool.handler!({ model: 'gpt-5-mini' }, {}); expect(result).toEqual({ ok: false, @@ -130,13 +195,190 @@ describe('resolve_llm tool', () => { }); }); + describe('free OpenAI credits', () => { + it('claims free OpenAI credits when no LLM credentials exist and the user is eligible', async () => { + const credentialProvider = makeProvider([]); + const modelLookup = makeModelLookup(); + const freeCredits = makeFreeCredits( + () => true, + async () => ({ credentialId: 'free-1', credentialName: 'n8n free OpenAI API credits' }), + ); + const tool = buildResolveLlmTool({ credentialProvider, modelLookup, freeCredits }); + const result = await tool.handler!({}, {}); + + expect(result).toEqual({ + ok: true, + provider: 'openai', + model: 'gpt-5-mini', + credentialId: 'free-1', + credentialName: 'n8n free OpenAI API credits', + claimedFreeOpenAiCredits: true, + }); + }); + + it('returns missing_credential when no LLM credentials exist and free credits are not eligible', async () => { + const credentialProvider = makeProvider([]); + const modelLookup = makeModelLookup(); + const freeCredits = makeFreeCredits(); + const tool = buildResolveLlmTool({ credentialProvider, modelLookup, freeCredits }); + const result = await tool.handler!({}, {}); + + expect(result).toEqual({ ok: false, reason: 'missing_credential', credentials: [] }); + expect(freeCredits.claim).not.toHaveBeenCalled(); + }); + + it('falls back to missing_credential when the free-credits claim fails', async () => { + const credentialProvider = makeProvider([]); + const modelLookup = makeModelLookup(); + const freeCredits = makeFreeCredits( + () => true, + async () => { + throw new Error('Already claimed'); + }, + ); + const tool = buildResolveLlmTool({ credentialProvider, modelLookup, freeCredits }); + const result = await tool.handler!({}, {}); + + expect(result).toEqual({ ok: false, reason: 'missing_credential', credentials: [] }); + }); + + it('claims free credits when openai is requested without a model and no openai credential exists', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'My Anthropic', type: 'anthropicApi' }, + ]); + const modelLookup = makeModelLookup(); + const freeCredits = makeFreeCredits( + () => true, + async () => ({ credentialId: 'free-1', credentialName: 'n8n free OpenAI API credits' }), + ); + const tool = buildResolveLlmTool({ credentialProvider, modelLookup, freeCredits }); + const result = await tool.handler!({ provider: 'openai' }, {}); + + expect(result).toEqual({ + ok: true, + provider: 'openai', + model: 'gpt-5-mini', + credentialId: 'free-1', + credentialName: 'n8n free OpenAI API credits', + claimedFreeOpenAiCredits: true, + }); + }); + + it('does not claim free credits when openai is requested with a specific model', async () => { + const credentialProvider = makeProvider([]); + const modelLookup = makeModelLookup(); + const freeCredits = makeFreeCredits(() => true); + const tool = buildResolveLlmTool({ credentialProvider, modelLookup, freeCredits }); + const result = await tool.handler!({ provider: 'openai', model: 'gpt-4.1' }, {}); + + expect(result).toEqual({ + ok: false, + reason: 'missing_credential', + provider: 'openai', + credentialType: 'openAiApi', + credentials: [], + }); + expect(freeCredits.claim).not.toHaveBeenCalled(); + }); + + it('does not claim free credits when a model is requested without a provider', async () => { + const credentialProvider = makeProvider([]); + const modelLookup = makeModelLookup(); + const freeCredits = makeFreeCredits(() => true); + const tool = buildResolveLlmTool({ credentialProvider, modelLookup, freeCredits }); + const result = await tool.handler!({ model: 'claude-sonnet-4-6' }, {}); + + expect(result).toEqual({ ok: false, reason: 'missing_credential', credentials: [] }); + expect(freeCredits.claim).not.toHaveBeenCalled(); + }); + }); + + describe('credentialId', () => { + it('resolves a specific credential when credentialId is passed', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'Personal OpenRouter', type: 'openRouterApi' }, + { id: 'c2', name: 'Work OpenRouter', type: 'openRouterApi' }, + ]); + const modelLookup = makeModelLookup(); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); + const result = await tool.handler!({ credentialId: 'c2' }, {}); + + expect(result).toEqual({ + ok: true, + provider: 'openrouter', + model: 'anthropic/claude-sonnet-4.6', + credentialId: 'c2', + credentialName: 'Work OpenRouter', + }); + }); + + it('returns unknown_credential for a credentialId that is not an LLM credential', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'My Anthropic', type: 'anthropicApi' }, + ]); + const modelLookup = makeModelLookup(); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); + const result = await tool.handler!({ credentialId: 'nope' }, {}); + + expect(result).toEqual({ + ok: false, + reason: 'unknown_credential', + credentialId: 'nope', + credentials: [{ id: 'c1', name: 'My Anthropic', type: 'anthropicApi' }], + }); + }); + }); + + describe('cross-provider auto-pick', () => { + it('auto-picks the highest-priority provider when multiple providers each have one credential', async () => { + const credentialProvider = makeProvider([ + { id: 'c1', name: 'My OpenAI', type: 'openAiApi' }, + { id: 'c2', name: 'My Anthropic', type: 'anthropicApi' }, + ]); + const modelLookup = makeModelLookup(); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); + const result = await tool.handler!({}, {}); + + expect(result).toEqual({ + ok: true, + provider: 'anthropic', + model: 'claude-sonnet-4-6', + credentialId: 'c2', + credentialName: 'My Anthropic', + autoPicked: true, + otherProviders: ['openai'], + }); + }); + + it('LLM_PROVIDER_PRIORITY covers every provider in LLM_PROVIDER_DEFAULTS', () => { + const definedProviders = new Set(Object.values(LLM_PROVIDER_DEFAULTS).map((d) => d.provider)); + expect(new Set(LLM_PROVIDER_PRIORITY)).toEqual(definedProviders); + }); + }); + describe('model validation against modelLookup', () => { it('skips lookup when no model is requested', async () => { const credentialProvider = makeProvider([ { id: 'c1', name: 'My Anthropic', type: 'anthropicApi' }, ]); const modelLookup = makeModelLookup(); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'anthropic' }, {}); expect(result).toEqual({ @@ -155,7 +397,11 @@ describe('resolve_llm tool', () => { { name: 'Command R+', value: 'command-r-plus' }, { name: 'Command R', value: 'command-r' }, ]); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'cohere', model: 'command-r-plus' }, {}); expect(result).toEqual({ @@ -176,7 +422,11 @@ describe('resolve_llm tool', () => { { name: 'Claude Haiku 4.5', value: 'claude-haiku-4-5-20250101' }, { name: 'Claude Sonnet 4.6', value: 'claude-sonnet-4-6' }, ]); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!( { provider: 'anthropic', model: 'CLAUDE-HAIKU-4-5-20250101' }, {}, @@ -200,7 +450,11 @@ describe('resolve_llm tool', () => { { name: 'Claude Haiku 4.5', value: 'claude-haiku-4-5-20250101' }, { name: 'Claude Sonnet 4.6', value: 'claude-sonnet-4-6' }, ]); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'anthropic', model: 'claude-haiku-4-5' }, {}); expect(result).toEqual({ @@ -220,7 +474,11 @@ describe('resolve_llm tool', () => { { name: 'Claude Haiku 4.5', value: 'claude-haiku-4-5-20250101' }, { name: 'Claude Sonnet 4.6', value: 'claude-sonnet-4-6-20251001' }, ]); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'anthropic', model: 'haiku 4.5' }, {}); expect(result).toEqual({ @@ -238,7 +496,11 @@ describe('resolve_llm tool', () => { ]); const available = [{ name: 'Claude Sonnet 4.6', value: 'claude-sonnet-4-6' }]; const modelLookup = makeModelLookup(async () => available); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'anthropic', model: 'gpt-9000' }, {}); expect(result).toEqual({ @@ -259,7 +521,11 @@ describe('resolve_llm tool', () => { { name: 'Claude Haiku 4.0', value: 'claude-haiku-4-0-20240101' }, { name: 'Claude Sonnet 4.6', value: 'claude-sonnet-4-6' }, ]); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'anthropic', model: 'haiku' }, {}); expect(result).toEqual({ @@ -281,7 +547,11 @@ describe('resolve_llm tool', () => { const modelLookup = makeModelLookup(async () => { throw new Error('credentials invalid'); }); - const tool = buildResolveLlmTool({ credentialProvider, modelLookup }); + const tool = buildResolveLlmTool({ + credentialProvider, + modelLookup, + freeCredits: makeFreeCredits(), + }); const result = await tool.handler!({ provider: 'anthropic', model: 'claude-haiku-4-5' }, {}); expect(result).toEqual({ diff --git a/packages/cli/src/modules/agents/builder/interactive/ask-credential.tool.ts b/packages/cli/src/modules/agents/builder/interactive/ask-credential.tool.ts index a4a90d7a06b..dc548832666 100644 --- a/packages/cli/src/modules/agents/builder/interactive/ask-credential.tool.ts +++ b/packages/cli/src/modules/agents/builder/interactive/ask-credential.tool.ts @@ -136,8 +136,11 @@ export function buildAskCredentialTool(deps: AskCredentialToolDeps): BuiltTool { return new Tool(ASK_CREDENTIAL_TOOL_NAME) .description( 'Show a credential picker card in the chat UI and suspend until the user selects ' + - 'a credential. Call ONCE per credential slot, BEFORE the write_config / patch_config ' + - 'that introduces the node tool. Returns { credentialId, credentialName, credentials } on success ' + + 'a credential. Call ONCE per credential slot. For an addition to an existing agent, ' + + 'call it before the write_config / patch_config that introduces the tool. Never call ' + + 'this during an initial build — follow the Initial Build rules in your system prompt; ' + + 'use it for additions to an existing agent and follow-up setup turns. ' + + 'Returns { credentialId, credentialName, credentials } on success ' + 'or { skipped: true } if the user skips credential setup so the tool can be added ' + 'without credentials. For node tools, copy the returned `credentials` object into `node.credentials`. Auto-resolves without ' + 'rendering a card when the agent has a chat channel configured whose credential matches the ' + diff --git a/packages/cli/src/modules/agents/builder/interactive/configure-channel.tool.ts b/packages/cli/src/modules/agents/builder/interactive/configure-channel.tool.ts index bbbe50026a0..b8b10573e6b 100644 --- a/packages/cli/src/modules/agents/builder/interactive/configure-channel.tool.ts +++ b/packages/cli/src/modules/agents/builder/interactive/configure-channel.tool.ts @@ -30,7 +30,7 @@ export function buildConfigureChannelTool(deps: ConfigureChannelToolDeps): Built 'list_integration_types and pass a returned `type` as `integrationType`; do not infer ' + 'channel names. Shows setup UI in chat where the user creates a new channel credential ' + 'or skips. The setup UI persists the connection, so use this for channel credentials ' + - 'instead of the credentials tool or config writes. Returns { connected: boolean }; if ' + + 'instead of the credentials tool or config writes. Returns { connected: boolean } (plus configMutated/agentId refresh metadata when connected); if ' + 'false, continue without the channel and do not re-prompt.', ) .input(configureChannelInputSchema) diff --git a/packages/cli/src/modules/agents/builder/interactive/finish-setup.tool.ts b/packages/cli/src/modules/agents/builder/interactive/finish-setup.tool.ts new file mode 100644 index 00000000000..945e72eb983 --- /dev/null +++ b/packages/cli/src/modules/agents/builder/interactive/finish-setup.tool.ts @@ -0,0 +1,470 @@ +import type { + BuiltTool, + CredentialListItem, + CredentialProvider, + InterruptibleToolContext, +} from '@n8n/agents'; +import { Tool } from '@n8n/agents/tool'; +import { + channelSuspendPayloadSchema, + credentialSuspendPayloadSchema, + interactionQuestionSchema, + questionAnswerSchema, + questionsSuspendPayloadSchema, + type InteractionQuestion, +} from '@n8n/api-types'; +import { nanoid } from 'nanoid'; +import { z } from 'zod'; + +import { BUILDER_TOOLS } from '../builder-tool-names'; + +/** Filters an already-fetched credential list down to one type, in the shape the setup cards need. */ +function credentialsOfType( + all: CredentialListItem[], + credentialType: string, +): Array<{ id: string; name: string }> { + return all.filter((c) => c.type === credentialType).map((c) => ({ id: c.id, name: c.name })); +} + +/** Compact publish-blocking validation issue, used to gate the channel phase. */ +export interface PublishBlockerIssue { + path: string; + code: string; +} + +export interface FinishSetupToolDeps { + credentialProvider: CredentialProvider; + agentId: string; + projectId: string; + isCredentialTypeKnown?: (credentialType: string) => boolean; + /** Credential ids of the agent's configured chat channel integrations — reused for a matching credential slot. */ + listIntegrationCredentialIds?: () => Promise; + /** Wraps `AgentIntegrationPersistenceService.listChatIntegrations()`. */ + listChatIntegrationTypes: () => string[]; + /** + * Publish-blocking validation issues on the agent's current (draft) config, + * excluding `integrations.*` paths. Checked before entering a channel + * phase — connecting a channel auto-publishes the agent, which would + * otherwise fail with a raw publish error whenever another part of the + * config (e.g. a skipped tool credential) is still invalid. + */ + getPublishBlockers: () => Promise; +} + +const finishSetupCredentialRequestInputSchema = z.object({ + credentialType: z.string().min(1), + purpose: z.string().min(1), + credentialSlot: z.string().optional(), +}); +type CredentialSlotInput = z.infer; + +const finishSetupChannelInputSchema = z.object({ + integrationType: z.string().min(1), +}); + +const finishSetupInputSchema = z + .object({ + questions: z.array(interactionQuestionSchema).optional(), + credentialRequests: z.array(finishSetupCredentialRequestInputSchema).optional(), + channels: z.array(finishSetupChannelInputSchema).optional(), + }) + .refine( + (v) => + (v.questions?.length ?? 0) + (v.credentialRequests?.length ?? 0) + (v.channels?.length ?? 0) > + 0, + { message: 'Pass at least one pending setup item.' }, + ); +type FinishSetupInput = z.infer; + +/** One resolved credential outcome per slot key — either a resolved credential or an explicit skip. */ +const credentialOutcomeSchema = z.union([ + z.object({ id: z.string(), name: z.string() }), + z.literal('skipped'), +]); + +/** + * A channel is connected (the setup card persisted it), skipped (the user + * dismissed the card), or blocked (its card was never shown because the + * agent could not be published yet). + */ +const channelOutcomeSchema = z.union([ + z.literal('connected'), + z.literal('skipped'), + z.literal('blocked'), +]); + +const questionsPhaseSchema = z.object({ kind: z.literal('questions') }); +const credentialsPhaseSchema = z.object({ + kind: z.literal('credentials'), + slots: z.array(finishSetupCredentialRequestInputSchema), +}); +const channelPhaseSchema = z.object({ + kind: z.literal('channel'), + integrationType: z.string(), +}); +const phaseDescriptorSchema = z.union([ + questionsPhaseSchema, + credentialsPhaseSchema, + channelPhaseSchema, +]); +type PhaseDescriptor = z.infer; + +const collectedSchema = z.object({ + answers: z.array(questionAnswerSchema).optional(), + credentials: z.record(credentialOutcomeSchema).optional(), + channels: z.record(channelOutcomeSchema).optional(), +}); +type Collected = z.infer; + +/** + * Chain state carried inside the suspend payload (a member of each phase's + * suspend schema, so it round-trips through the builder checkpoint) and + * stripped by instance AI's cascade before the FE ever sees it — the FE + * routes purely on the presence of `inputType`/`credentialRequests`/ + * `channelConfig`, identical to the single-purpose interactive tools. + */ +const chainStateSchema = z.object({ + currentPhase: phaseDescriptorSchema, + remainingPhases: z.array(phaseDescriptorSchema), + collected: collectedSchema, + totalPhases: z.number(), +}); +type ChainState = z.infer; + +const finishSetupSuspendSchema = z.union([ + questionsSuspendPayloadSchema.extend({ finishSetupChain: chainStateSchema }), + credentialSuspendPayloadSchema.extend({ finishSetupChain: chainStateSchema }), + channelSuspendPayloadSchema.extend({ finishSetupChain: chainStateSchema }), +]); +type FinishSetupSuspendPayload = z.infer; + +/** + * Deliberately a single permissive object, not a union — the three phases' + * resume shapes overlap enough (e.g. questions/credentials and channel both + * carry an optional `approved`) that a union would ambiguously match the + * wrong arm. The handler always knows which phase a resume belongs to from + * `ctx.suspendPayload.finishSetupChain`, so shape ambiguity here is harmless. + */ +const finishSetupResumeSchema = z.object({ + approved: z.boolean().optional(), + answers: z.array(questionAnswerSchema).optional(), + credentials: z.record(z.string()).optional(), + skipped: z.boolean().optional(), +}); +type FinishSetupResumeData = z.infer; + +type FinishSetupCtx = InterruptibleToolContext; + +interface FinishSetupToolResult extends Collected { + completed: true; + /** Present only when a channel phase was skipped because the agent could not be published yet. */ + publishBlockedIssues?: PublishBlockerIssue[]; +} + +/** Throws for any credential request whose type isn't recognized. */ +function validateCredentialTypes(input: FinishSetupInput, deps: FinishSetupToolDeps): void { + for (const request of input.credentialRequests ?? []) { + if (deps.isCredentialTypeKnown && !deps.isCredentialTypeKnown(request.credentialType)) { + throw new Error( + `Unknown credential type "${request.credentialType}". Use an exact n8n credential type name.`, + ); + } + } +} + +/** Throws for any requested channel whose type isn't a known chat integration. */ +function validateChannelTypes(input: FinishSetupInput, deps: FinishSetupToolDeps): void { + const availableChannelTypes = deps.listChatIntegrationTypes(); + for (const channel of input.channels ?? []) { + if (!availableChannelTypes.includes(channel.integrationType)) { + const availableMessage = availableChannelTypes.length + ? ` Available: ${availableChannelTypes.join(', ')}.` + : ' No chat channels are currently available.'; + throw new Error( + `Unsupported chat channel "${channel.integrationType}". Call list_integration_types ` + + 'and choose a returned type.' + + availableMessage, + ); + } + } +} + +/** + * Validate input, then auto-resolve every credential slot using the same + * rules as ask_credential (matching channel credential first, then a single + * existing credential of the type). Slots that cannot be auto-resolved + * become a phase. Phase order is fixed: questions, then credentials, then + * one channel phase per requested channel — channels always run last since + * their card persists the connection immediately via REST. + */ +async function computeInitialPlan( + input: FinishSetupInput, + deps: FinishSetupToolDeps, +): Promise<{ phases: PhaseDescriptor[]; collected: Collected }> { + validateCredentialTypes(input, deps); + validateChannelTypes(input, deps); + + const collected: Collected = {}; + const pendingSlots: CredentialSlotInput[] = []; + + if (input.credentialRequests?.length) { + const integrationCredentialIds = (await deps.listIntegrationCredentialIds?.()) ?? []; + const all = await deps.credentialProvider.list(); + const credentials: Record> = {}; + + for (const slot of input.credentialRequests) { + const key = slot.credentialSlot ?? slot.credentialType; + const existingCredentials = credentialsOfType(all, slot.credentialType); + const channelMatch = existingCredentials.find((credential) => + integrationCredentialIds.includes(credential.id), + ); + const autoResolved = + channelMatch ?? (existingCredentials.length === 1 ? existingCredentials[0] : undefined); + + if (autoResolved) { + credentials[key] = autoResolved; + } else { + pendingSlots.push(slot); + } + } + + if (Object.keys(credentials).length > 0) collected.credentials = credentials; + } + + const phases: PhaseDescriptor[] = []; + if (input.questions?.length) phases.push({ kind: 'questions' }); + if (pendingSlots.length > 0) phases.push({ kind: 'credentials', slots: pendingSlots }); + for (const channel of input.channels ?? []) { + phases.push({ kind: 'channel', integrationType: channel.integrationType }); + } + + return { phases, collected }; +} + +/** Merge a phase's resume data into the running `collected` result. Any dismissal/denial marks that phase's items skipped rather than aborting the chain. */ +async function mergeResumeIntoCollected( + phase: PhaseDescriptor, + resumeData: FinishSetupResumeData | undefined, + previous: Collected, + deps: FinishSetupToolDeps, +): Promise { + if (phase.kind === 'questions') { + return { ...previous, answers: resumeData?.answers ?? [] }; + } + + if (phase.kind === 'channel') { + const channels = { ...(previous.channels ?? {}) }; + channels[phase.integrationType] = resumeData?.approved ? 'connected' : 'skipped'; + return { ...previous, channels }; + } + + const all = await deps.credentialProvider.list(); + const credentials = { ...(previous.credentials ?? {}) }; + for (const slot of phase.slots) { + const key = slot.credentialSlot ?? slot.credentialType; + const credentialId = resumeData?.credentials?.[slot.credentialType]; + credentials[key] = credentialId + ? { + id: credentialId, + name: + credentialsOfType(all, slot.credentialType).find((c) => c.id === credentialId)?.name ?? + credentialId, + } + : 'skipped'; + } + return { ...previous, credentials }; +} + +/** + * Before entering a channel phase, verify the agent can currently be + * published — connecting a channel auto-publishes it, so an already-invalid + * config (e.g. a skipped tool credential) would otherwise surface as a raw + * publish error from the channel card's REST call instead of a clear message + * here. Channel phases are always the trailing, consecutive phases (see + * computeInitialPlan), so once `phase` is a channel phase, every phase in + * `remainingPhases` is one too. + */ +async function checkChannelPublishability( + phase: PhaseDescriptor, + remainingPhases: PhaseDescriptor[], + collected: Collected, + deps: FinishSetupToolDeps, +): Promise { + if (phase.kind !== 'channel') return undefined; + + const publishBlockedIssues = await deps.getPublishBlockers(); + if (publishBlockedIssues.length === 0) return undefined; + + const channels = { ...(collected.channels ?? {}) }; + for (const blockedPhase of [phase, ...remainingPhases]) { + if (blockedPhase.kind === 'channel') channels[blockedPhase.integrationType] = 'blocked'; + } + return { completed: true, ...collected, channels, publishBlockedIssues }; +} + +/** Suspend for the given phase, carrying the remaining plan forward in the chain state. */ +async function suspendForPhase(params: { + phase: PhaseDescriptor; + remainingPhases: PhaseDescriptor[]; + collected: Collected; + totalPhases: number; + phaseNumber: number; + questions: InteractionQuestion[] | undefined; + deps: FinishSetupToolDeps; + ctx: FinishSetupCtx; +}): Promise { + const { phase, remainingPhases, collected, totalPhases, phaseNumber, questions, deps, ctx } = + params; + const finishSetupChain: ChainState = { + currentPhase: phase, + remainingPhases, + collected, + totalPhases, + }; + const message = `Finish setup (${phaseNumber}/${totalPhases})`; + + if (phase.kind === 'questions') { + return await ctx.suspend({ + requestId: nanoid(), + message, + severity: 'info' as const, + inputType: 'questions' as const, + questions: questions ?? [], + finishSetupChain, + }); + } + + if (phase.kind === 'channel') { + return await ctx.suspend({ + requestId: nanoid(), + message: `Set up the ${phase.integrationType} channel`, + severity: 'info' as const, + channelConfig: { integrationType: phase.integrationType, agentId: deps.agentId }, + projectId: deps.projectId, + finishSetupChain, + }); + } + + const all = await deps.credentialProvider.list(); + const seenTypes = new Set(); + const credentialRequests: Array<{ + credentialType: string; + reason: string; + existingCredentials: Array<{ id: string; name: string }>; + }> = []; + for (const slot of phase.slots) { + if (seenTypes.has(slot.credentialType)) continue; + seenTypes.add(slot.credentialType); + credentialRequests.push({ + credentialType: slot.credentialType, + reason: slot.purpose, + existingCredentials: credentialsOfType(all, slot.credentialType), + }); + } + return await ctx.suspend({ + requestId: nanoid(), + message, + severity: 'info' as const, + credentialRequests, + credentialFlow: { stage: 'generic' as const }, + finishSetupChain, + }); +} + +async function startPlan( + input: FinishSetupInput, + ctx: FinishSetupCtx, + deps: FinishSetupToolDeps, +): Promise { + const { phases, collected } = await computeInitialPlan(input, deps); + if (phases.length === 0) { + return { completed: true, ...collected }; + } + + const [currentPhase, ...remainingPhases] = phases; + const blocked = await checkChannelPublishability(currentPhase, remainingPhases, collected, deps); + if (blocked) return blocked; + + return await suspendForPhase({ + phase: currentPhase, + remainingPhases, + collected, + totalPhases: phases.length, + phaseNumber: 1, + questions: input.questions, + deps, + ctx, + }); +} + +async function resumePlan( + input: FinishSetupInput, + ctx: FinishSetupCtx, + deps: FinishSetupToolDeps, +): Promise { + // Guarded by the caller: ctx.suspendPayload is set whenever this branch runs. + const chain = ctx.suspendPayload!.finishSetupChain; + const collected = await mergeResumeIntoCollected( + chain.currentPhase, + ctx.resumeData, + chain.collected, + deps, + ); + + if (chain.remainingPhases.length === 0) { + return { completed: true, ...collected }; + } + + const [nextPhase, ...restPhases] = chain.remainingPhases; + const blocked = await checkChannelPublishability(nextPhase, restPhases, collected, deps); + if (blocked) return blocked; + + return await suspendForPhase({ + phase: nextPhase, + remainingPhases: restPhases, + collected, + totalPhases: chain.totalPhases, + phaseNumber: chain.totalPhases - restPhases.length, + questions: input.questions, + deps, + ctx, + }); +} + +export function buildFinishSetupTool(deps: FinishSetupToolDeps): BuiltTool { + return new Tool(BUILDER_TOOLS.FINISH_SETUP) + .description( + 'Collect everything still needed to finish the initial build in ONE guided flow: open ' + + 'questions (including the model choice), credential slots, and chat-channel ' + + 'connections. Call it at most once, only in the trailing step of an initial build ' + + 'when only blocked tasks remain, and never together with another interactive tool. ' + + 'It shows the question and credential cards back-to-back without returning control ' + + 'between them, then one card per requested channel (always last, since connecting a ' + + 'channel needs credentials to already be resolved) — but a channel card shows ' + + 'in-chain only when the agent is already publishable when its phase is reached. Pass ' + + '`channels` with a returned `type` from list_integration_types, one entry per channel ' + + 'to connect; do not infer channel names. Connecting a channel publishes the agent, ' + + 'and answers/credentials collected by this call are NOT applied to the config ' + + 'mid-flow — so if the agent still has publish-blocking issues once a channel phase ' + + 'is reached (expected whenever this same call is still collecting the model or a ' + + 'required credential), that ' + + 'phase\'s card is never shown — its outcome is `"blocked"` instead of `"connected"`/' + + '`"skipped"`, and the result carries `publishBlockedIssues`. Resolve those issues first ' + + '(patch in the credentials/model this same call already collected), then call ' + + 'configure_channel directly for each blocked channel. Returns ' + + '{ completed, answers, credentials, channels, publishBlockedIssues } (plus configMutated/agentId refresh metadata when completed): resolve the model ' + + 'answer with resolve_llm, copy returned credential ids into the config, and verify MCP ' + + 'servers with them. Auto-resolves credential slots that match an existing single ' + + 'credential or the connected channel credential.', + ) + .input(finishSetupInputSchema) + .suspend(finishSetupSuspendSchema) + .resume(finishSetupResumeSchema) + .handler(async (input: FinishSetupInput, ctx: FinishSetupCtx) => { + if (ctx.suspendPayload) { + return await resumePlan(input, ctx, deps); + } + return await startPlan(input, ctx, deps); + }) + .build(); +} diff --git a/packages/cli/src/modules/agents/builder/interactive/index.ts b/packages/cli/src/modules/agents/builder/interactive/index.ts index 2c4c6ef1068..9e73fd7780d 100644 --- a/packages/cli/src/modules/agents/builder/interactive/index.ts +++ b/packages/cli/src/modules/agents/builder/interactive/index.ts @@ -1,4 +1,5 @@ export { buildAskCredentialTool, buildAskEmbeddingCredentialTool } from './ask-credential.tool'; export { buildAskQuestionsTool } from './ask-questions.tool'; export { buildConfigureChannelTool } from './configure-channel.tool'; +export { buildFinishSetupTool } from './finish-setup.tool'; export { buildResolveLlmTool } from './resolve-llm.tool'; diff --git a/packages/cli/src/modules/agents/builder/interactive/llm-provider-defaults.ts b/packages/cli/src/modules/agents/builder/interactive/llm-provider-defaults.ts index 3e807dce9fe..7b0c02c5ca6 100644 --- a/packages/cli/src/modules/agents/builder/interactive/llm-provider-defaults.ts +++ b/packages/cli/src/modules/agents/builder/interactive/llm-provider-defaults.ts @@ -65,3 +65,18 @@ export const LLM_PROVIDER_DEFAULTS: Record = { defaultModel: 'anthropic/claude-sonnet-4.6', }, }; + +/** Order in which resolve_llm auto-picks a provider when credentials span multiple providers. */ +export const LLM_PROVIDER_PRIORITY: string[] = [ + 'anthropic', + 'openai', + 'google', + 'mistral', + 'xai', + 'groq', + 'deepseek', + 'cohere', + 'openrouter', + 'nvidia', + 'vercel', +]; 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 915fd076377..2d088cf8c79 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 @@ -4,7 +4,11 @@ import { isModelDiscoveryProvider } from '@n8n/ai-utilities/model-discovery'; import { z } from 'zod'; import { BUILDER_TOOLS } from '../builder-tool-names'; -import { LLM_PROVIDER_DEFAULTS, type LlmProviderDefault } from './llm-provider-defaults'; +import { + LLM_PROVIDER_DEFAULTS, + LLM_PROVIDER_PRIORITY, + type LlmProviderDefault, +} from './llm-provider-defaults'; export interface ModelLookup { list( @@ -14,13 +18,40 @@ export interface ModelLookup { ): Promise>; } +/** Provisions free OpenAI credits on demand for a zero-credential builder session. */ +export interface FreeCreditsProvisioner { + isEligible(): boolean | Promise; + claim(): Promise<{ credentialId: string; credentialName: string }>; +} + export interface ResolveLlmToolDeps { credentialProvider: CredentialProvider; modelLookup: ModelLookup; + freeCredits: FreeCreditsProvisioner; } type LlmCredentialEntry = [credentialType: string, defaults: LlmProviderDefault]; +const FREE_CREDITS_MODEL = 'gpt-5-mini'; + +/** Silently claims free OpenAI credits if eligible; never throws. */ +async function tryClaimFreeCredits(freeCredits: FreeCreditsProvisioner) { + try { + if (!(await freeCredits.isEligible())) return null; + const { credentialId, credentialName } = await freeCredits.claim(); + return { + ok: true as const, + provider: 'openai', + model: FREE_CREDITS_MODEL, + credentialId, + credentialName, + claimedFreeOpenAiCredits: true as const, + }; + } catch { + return null; + } +} + function findProviderDefault(provider: string): LlmCredentialEntry | undefined { const requestedProvider = provider.trim(); return Object.entries(LLM_PROVIDER_DEFAULTS).find( @@ -92,13 +123,23 @@ export function buildResolveLlmTool(deps: ResolveLlmToolDeps): BuiltTool { return new Tool(BUILDER_TOOLS.RESOLVE_LLM) .description( 'Resolve the agent main LLM without showing a picker. ' + - 'Only call this when the user explicitly names a provider or model — do NOT call it ' + - 'at the start of a conversation or proactively for fresh agents. ' + + 'For fresh agents, call it once, silently, before the first config write to detect existing ' + + 'credentials — with provider/model when the user named them, otherwise without arguments. ' + + 'Also call it whenever the user names or changes a provider or model. ' + 'If provider is given, resolves only that provider; if model is omitted, uses the ' + 'provider default model. For "Anthropic via OpenRouter", pass provider="openrouter" ' + 'and omit model unless the user named a concrete OpenRouter model id. Returns ok=false ' + - 'when credentials are missing, unsupported, or ambiguous; use ask_questions to let the ' + - 'user choose, then call resolve_llm again with the choice.', + 'when credentials are missing, unsupported, or ambiguous — during an initial build, do not ' + + 'ask; keep building with model "" and include the model choice in the trailing ' + + 'finish_setup call, then call resolve_llm again with the answer. For a model ' + + 'change on an existing agent, ask immediately and keep the current model and credential until the new one resolves. ' + + 'When no matching credential exists and the user is eligible for free OpenAI credits, the tool ' + + 'claims them automatically and resolves to openai/gpt-5-mini — the result carries ' + + 'claimedFreeOpenAiCredits: true; tell the user free OpenAI credits were set up. When multiple ' + + 'providers each have one credential, the tool auto-picks the recommended provider — the result ' + + 'carries autoPicked: true and otherProviders; state the pick as changeable, do not ask to confirm it. ' + + 'When the user picks between multiple credentials of one provider, pass the picked credentialId ' + + 'from the earlier ambiguous result.', ) .input( z.object({ @@ -112,78 +153,147 @@ export function buildResolveLlmTool(deps: ResolveLlmToolDeps): BuiltTool { .describe( 'Requested model without the selected provider prefix. For OpenRouter use the routed id, e.g. "anthropic/claude-sonnet-4.6".', ), + credentialId: z + .string() + .optional() + .describe( + 'Credential id picked by the user from an earlier ambiguous resolve_llm result.', + ), }), ) - .handler(async ({ provider, model }: { provider?: string; model?: string }) => { - const all = await deps.credentialProvider.list(); - const llmCredentials = all.filter((credential) => LLM_PROVIDER_DEFAULTS[credential.type]); + .handler( + async ({ + provider, + model, + credentialId, + }: { + provider?: string; + model?: string; + credentialId?: string; + }) => { + const all = await deps.credentialProvider.list(); + const llmCredentials = all.filter((credential) => LLM_PROVIDER_DEFAULTS[credential.type]); - if (provider) { - const providerEntry = findProviderDefault(provider); - if (!providerEntry) { - return { - ok: false as const, - reason: 'unsupported_provider' as const, - provider, - supportedProviders: Object.values(LLM_PROVIDER_DEFAULTS).map( - (defaults) => defaults.provider, - ), - }; - } - - const [credentialType, defaults] = providerEntry; - const matchingCredentials = llmCredentials.filter( - (credential) => credential.type === credentialType, - ); - - if (matchingCredentials.length === 1) { - const credential = matchingCredentials[0]; + if (credentialId) { + const credential = llmCredentials.find((c) => c.id === credentialId); + if (!credential) { + return { + ok: false as const, + reason: 'unknown_credential' as const, + credentialId, + credentials: llmCredentials.map((c) => ({ + id: c.id, + name: c.name, + type: c.type, + })), + }; + } + const defaults = LLM_PROVIDER_DEFAULTS[credential.type]; if (model?.trim()) { return await resolveModelAgainstLookup(credential, defaults, model, deps.modelLookup); } return toLlmResolution(credential, defaults); } + if (provider) { + const providerEntry = findProviderDefault(provider); + if (!providerEntry) { + return { + ok: false as const, + reason: 'unsupported_provider' as const, + provider, + supportedProviders: Object.values(LLM_PROVIDER_DEFAULTS).map( + (defaults) => defaults.provider, + ), + }; + } + + const [credentialType, defaults] = providerEntry; + const matchingCredentials = llmCredentials.filter( + (credential) => credential.type === credentialType, + ); + + if (matchingCredentials.length === 1) { + const credential = matchingCredentials[0]; + if (model?.trim()) { + return await resolveModelAgainstLookup(credential, defaults, model, deps.modelLookup); + } + return toLlmResolution(credential, defaults); + } + + if ( + matchingCredentials.length === 0 && + defaults.provider === 'openai' && + !model?.trim() + ) { + const claimed = await tryClaimFreeCredits(deps.freeCredits); + if (claimed) return claimed; + } + + return { + ok: false as const, + reason: + matchingCredentials.length === 0 + ? ('missing_credential' as const) + : ('ambiguous_credential' as const), + provider: defaults.provider, + credentialType, + credentials: matchingCredentials.map((credential) => ({ + id: credential.id, + name: credential.name, + })), + }; + } + + if (llmCredentials.length === 1) { + const credential = llmCredentials[0]; + const defaults = LLM_PROVIDER_DEFAULTS[credential.type]; + if (model?.trim()) { + return await resolveModelAgainstLookup(credential, defaults, model, deps.modelLookup); + } + return toLlmResolution(credential, defaults); + } + + if (llmCredentials.length === 0 && !model?.trim()) { + const claimed = await tryClaimFreeCredits(deps.freeCredits); + if (claimed) return claimed; + } + + if (llmCredentials.length > 1 && !model?.trim()) { + const byProvider = new Map(); + for (const credential of llmCredentials) { + const providerName = LLM_PROVIDER_DEFAULTS[credential.type].provider; + byProvider.set(providerName, [...(byProvider.get(providerName) ?? []), credential]); + } + + const topProvider = LLM_PROVIDER_PRIORITY.find((candidate) => byProvider.has(candidate)); + const topCredentials = topProvider ? byProvider.get(topProvider) : undefined; + if (topProvider && topCredentials?.length === 1) { + return { + ...toLlmResolution(topCredentials[0], LLM_PROVIDER_DEFAULTS[topCredentials[0].type]), + autoPicked: true as const, + otherProviders: [...byProvider.keys()].filter((other) => other !== topProvider), + }; + } + } + return { ok: false as const, reason: - matchingCredentials.length === 0 + llmCredentials.length === 0 ? ('missing_credential' as const) - : ('ambiguous_credential' as const), - provider: defaults.provider, - credentialType, - credentials: matchingCredentials.map((credential) => ({ - id: credential.id, - name: credential.name, - })), + : ('ambiguous_provider_or_credential' as const), + credentials: llmCredentials.map((credential) => { + const defaults = LLM_PROVIDER_DEFAULTS[credential.type]; + return { + id: credential.id, + name: credential.name, + type: credential.type, + provider: defaults.provider, + }; + }), }; - } - - if (llmCredentials.length === 1) { - const credential = llmCredentials[0]; - const defaults = LLM_PROVIDER_DEFAULTS[credential.type]; - if (model?.trim()) { - return await resolveModelAgainstLookup(credential, defaults, model, deps.modelLookup); - } - return toLlmResolution(credential, defaults); - } - - return { - ok: false as const, - reason: - llmCredentials.length === 0 - ? ('missing_credential' as const) - : ('ambiguous_provider_or_credential' as const), - credentials: llmCredentials.map((credential) => { - const defaults = LLM_PROVIDER_DEFAULTS[credential.type]; - return { - id: credential.id, - name: credential.name, - type: credential.type, - provider: defaults.provider, - }; - }), - }; - }) + }, + ) .build(); } diff --git a/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts index 3813d871c92..4d0655b8bf5 100644 --- a/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts @@ -34,17 +34,18 @@ ${getSchemaReferenceSection()} ### Recipes -#### Create Or Replace A Fresh Runnable Agent +#### Create A Fresh Agent Draft -- Requires \`name\`, \`model\`, \`credential\`, and \`instructions\`. +- Requires \`name\` and \`instructions\`. +- Use the model and credential from \`resolve_llm\` when resolved; while LLM + setup is pending, write \`model: ""\` and omit \`credential\`. - Keep \`tools\` and \`skills\` arrays if present. Good minimal shape: \`\`\`json { "name": "Support assistant", - "model": "openrouter/openai/gpt-5.5", - "credential": "", + "model": "", "instructions": "Help the user with support questions.", "tools": [], "skills": [] @@ -139,7 +140,7 @@ Bad: replacing \`config\` while dropping unrelated settings - Removing an integration means deleting its entry from \`integrations[]\`; do not call \`configure_channel\` for removal. - Model-only changes must preserve existing Brave or SearXNG \`config.webSearch\`. -- Empty, placeholder, or guessed \`instructions\` values are rejected; ask for details instead. +- Empty or placeholder \`instructions\` values are rejected; derive real instructions from the stated goal instead. ### Verify 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 6a037be4787..18ba71f0f82 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 { AgentModelSchema, RunnableAgentJsonConfigSchema } from '@n8n/api-types'; +import { AgentJsonConfigSchema, 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 = RunnableAgentJsonConfigSchema.extend({ +const BuilderPromptAgentJsonConfigSchema = AgentJsonConfigSchema.extend({ memory: BuilderPromptMemoryConfigSchema.optional(), }); @@ -49,8 +49,8 @@ export function getConfigRulesSection(): string { return `\ #### Agent Config Rules -- \`model\` must be "provider/model-name". -- \`credential\` must be the id returned by \`resolve_llm\`. +- \`model\` must be "provider/model-name", or \`""\` while LLM setup is pending. +- A non-empty \`credential\` must be the id returned by \`resolve_llm\`. - Sub-agent configuration lives at top level under \`subAgents\`. Load \`agent-builder-sub-agents\` before adding refs or changing \`subAgents.maxChildren\`. @@ -62,8 +62,9 @@ export function getConfigRulesSection(): string { \`"1h"\`) when the user asks to tune cache duration; OpenAI has no sub-config. - \`config.maxIterations\` caps the number of agent loop iterations per run. Do not set or change this unless the user explicitly asks. -- Fresh agents need a real model, credential, and instructions - before config is written.`; +- Fresh agents need real \`instructions\` before config is written. \`model\` + may be \`""\` and \`credential\` omitted in a draft while LLM setup is + pending; fill both from a \`resolve_llm\` result before publishing.`; } export function getSchemaReferenceSection(): string { diff --git a/packages/cli/src/modules/agents/builder/prompts/initial-build.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/initial-build.prompt.ts new file mode 100644 index 00000000000..7e8bb2222fb --- /dev/null +++ b/packages/cli/src/modules/agents/builder/prompts/initial-build.prompt.ts @@ -0,0 +1,67 @@ +/** + * Canonical initial-build contract. Other prompt surfaces (workflow steps, + * interactive-tool rules, skills, tool descriptions) state only their unique + * mechanics and reference this section instead of restating the rules. + */ +export const INITIAL_BUILD_SECTION = `\ +## Initial Build + +"Initial build" means the first build pass on a fresh agent. Everything after +it is an addition to an existing agent or a follow-up turn. + +During an initial build: + +- NEVER suspend mid-build on an interactive tool (\`ask_questions\`, + \`ask_credential\`, \`ask_embedding_credential\`, \`configure_channel\`). Build + everything as a draft first; the only allowed suspends are the single + trailing \`finish_setup\` call and, when it reports a blocked channel, the + immediate \`configure_channel\` follow-up described below. +- Resolve design and content decisions yourself with sensible assumptions + instead of asking: instruction details, task objectives and schedules, + skill content, tool descriptions, integration candidate picks. Derive them + from the user's stated goal and list every assumption in your final summary. +- Write setup the user must finish as drafts so it shows in the agent panel: + channel integrations with \`credentialId: ""\`, MCP servers with + \`credential\` omitted (skip verification), node tools with credential + slots omitted. Leave Episodic Memory disabled while its credential is + missing. +- Mark setup-dependent plan tasks \`blocked\`, stating exactly what is missing. +- When only blocked tasks remain, call \`finish_setup\` ONCE with everything + pending: the model choice and open decisions as \`questions\`, one + \`credentialRequests\` entry per credential slot, and one \`channels\` entry + per drafted channel integration — it connects or skips each channel itself, + always as the last cards in the flow. Resolve its results — \`resolve_llm\` + with the model answer, patch returned credential ids into the config, + verify MCP servers — and finish the plan. +- If \`finish_setup\` reports a channel as \`'blocked'\` (its + \`publishBlockedIssues\` field lists why — the agent could not be published + yet, so the card never showed), first apply everything \`finish_setup\` + already collected: patch returned credential ids into the config and + resolve the model with \`resolve_llm\`. If that resolves every reported + issue (nothing was skipped), call \`configure_channel\` once per blocked + channel as an immediate follow-up. If anything remains skipped or + unresolved, do not call \`configure_channel\`; leave it for the closing + checklist instead. +- After \`finish_setup\` and any follow-up \`configure_channel\` calls, end + your reply with a short setup checklist for whatever remains — any + skipped, dismissed, or still-blocked items only — one line per item naming + where to complete it in the agent panel (channels: the channel chip opens + the setup modal), plus the offer to do it here in chat. +- Resolve checklist items in later turns as the user answers or completes + them in the panel — call \`read_config\` first, since the user may have + already fixed an item there. + +Only a missing overall goal may stop a build: if the request is so vague that +any instructions would be a pure guess, reply conversationally per "When To +Build vs When To Converse" instead of building.`; + +/** + * Shared one-line deferral interpolated by skills and tool text at their + * point of use. Kept deliberately agnostic about how the deferred setup is + * ultimately resolved (today: a closing checklist) so this sentence and every + * skill that embeds it stay unchanged if the resolution mechanism changes — + * only INITIAL_BUILD_SECTION's ending and its two references in + * agents-builder-prompts.ts would need to. + */ +export const INITIAL_BUILD_NOTE = + 'During an initial build, never suspend mid-build: defer pending setup to the end of the build, per the Initial Build rules in your system prompt.'; diff --git a/packages/cli/src/modules/agents/builder/prompts/llm-selection.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/llm-selection.prompt.ts index 3c9422398b4..da034d3bd69 100644 --- a/packages/cli/src/modules/agents/builder/prompts/llm-selection.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/llm-selection.prompt.ts @@ -12,14 +12,17 @@ Use this to resolve the target agent's main \`model\` and \`credential\`. ### Workflow -1. Use \`resolve_llm\` when the request contains enough provider/model detail, otherwise ask via \`ask_questions\` and call \`resolve_llm\` with the answer. -2. If \`resolve_llm\` succeeds, persist \`model = "{provider}/{model}"\` and \`credential = credentialId\`. +1. For fresh agents, call \`resolve_llm\` once, silently, before the first config write — with provider/model when the user named them, otherwise with no arguments. +2. If \`resolve_llm\` succeeds, persist \`model = "{provider}/{model}"\` and \`credential = credentialId\`. If the result has \`claimedFreeOpenAiCredits: true\`, tell the user you set them up with free OpenAI credits. If it has \`autoPicked: true\`, tell the user which provider and model you picked and that they can ask to change it — do not ask for confirmation and do not raise a trailing model question. 3. If the user asks to pick, change, confirm, or configure a model or main credential, ask via \`ask_questions\`; do not ask in prose. -4. If \`resolve_llm\` reports missing or ambiguous credentials/provider, ask via \`ask_questions\` then retry \`resolve_llm\` with the answer. +4. During an initial build, if \`resolve_llm\` reports missing or ambiguous credentials/provider, do not ask: mark the model task \`blocked\`, keep building with \`model: ""\` and no \`credential\`, and include the model choice as a question in the trailing \`finish_setup\` call — for ambiguity between multiple credentials of one provider, use the credential names from the resolve_llm result as the question's options — when it resolves, call \`resolve_llm\` with the answer (pass \`credentialId\` when the user picked a specific credential) and patch \`/model\` and \`/credential\` — after \`read_config\`, since the user may have already set the model in the panel. For a model change on an existing agent, ask immediately instead, and never write \`model: ""\` over an existing model — keep the current model and credential until the new one is resolved. 5. If \`resolve_llm\` reports \`unknown_model\`, retry with a plausible returned model value or ask via \`ask_questions\`. +6. If the model is still unresolved when the user asks to run or publish the agent, leave the draft with \`model: ""\`, do not guess a model, and tell the user the agent needs a model and credential first — in the panel or here in chat. If they dismiss a model-change question on an existing agent, keep the current model and credential unchanged. ### Rules +- Do not enable \`config.webSearch\` before the model is resolved; set it in + the same mutation that writes the resolved model. - Only OpenAI and Anthropic models support native web search. Use native web search by default for those providers only, and only for fresh agents or agents with no existing \`config.webSearch\`. Persist diff --git a/packages/cli/src/modules/agents/builder/prompts/memory.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/memory.prompt.ts index aa7cc969aa1..bc73566e83e 100644 --- a/packages/cli/src/modules/agents/builder/prompts/memory.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/memory.prompt.ts @@ -35,5 +35,5 @@ disabling memory. Do not load it for ordinary fresh-agent creation. ### Verify -- Fresh runnable agents have enabled n8n memory unless explicitly disabled. -- Fresh runnable agents set \`observationalMemory.enabled\` to \`true\` unless explicitly disabled.`; +- Fresh agents have enabled n8n memory unless explicitly disabled. +- Fresh agents set \`observationalMemory.enabled\` to \`true\` unless explicitly disabled.`; diff --git a/packages/cli/src/modules/agents/builder/prompts/planner-todos.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/planner-todos.prompt.ts new file mode 100644 index 00000000000..a9591463313 --- /dev/null +++ b/packages/cli/src/modules/agents/builder/prompts/planner-todos.prompt.ts @@ -0,0 +1,26 @@ +/** + * Builder-specific model-visible text for the SDK's `createPlannerTodosTool`. + * The SDK ships domain-neutral defaults; these strings keep the agent + * builder's `write_todos` tool description/instructions unchanged from + * before the SDK text was made generic. + */ +export const BUILDER_PLANNER_TODOS_DESCRIPTION = + 'Create or update a structured task list for the current build. Use it to decompose the work, ' + + 'track progress, and record which tasks are blocked on user input. Use it for every build or ' + + 'change request. This tool only updates the task list; it does not perform work or ask the user.'; + +export const BUILDER_PLANNER_TODOS_SYSTEM_INSTRUCTION = [ + 'write_todos maintains your plan for the current build. It never asks the user or performs work itself.', + 'WHEN TO USE write_todos:', + '- Every request that builds or changes the agent, before other tool calls.', + '- You need to track progress or record tasks blocked on user input.', + 'WHEN NOT TO USE write_todos:', + '- Purely conversational replies with no build work.', + 'HOW TO USE write_todos:', + '- Write concrete, self-contained tasks; mark the first active task in_progress immediately.', + '- Mark a task blocked when it cannot proceed without user input, and state exactly what input is missing in the task content.', + '- Marking a task blocked IS the action for now. Do not ask the user about it while any non-blocked task remains; continue with unblocked tasks.', + '- When only blocked tasks remain: during an initial build, follow the Initial Build rules (call finish_setup); otherwise end your turn with a summary of what is missing, and unblock and finish the tasks in later turns as the user provides input.', + '- Update task status as soon as work completes; do not batch completions at the end.', + '- Do not call write_todos multiple times in parallel; send one full list update at a time.', +].join('\n'); diff --git a/packages/cli/src/modules/agents/builder/prompts/tools.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/tools.prompt.ts index 313b168d552..7b7d6b2e8bf 100644 --- a/packages/cli/src/modules/agents/builder/prompts/tools.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/tools.prompt.ts @@ -12,9 +12,9 @@ Use this guidance before calling \`resolve_integration\`, \`search_nodes\`, \`search_mcp_servers\`, \`get_node_types\`, \`build_custom_tool\`, or adding, changing, or removing entries in \`tools[]\` / \`mcpServers\` / \`providerTools\`. -For an external product, first decide whether it is the target agent's chat or -trigger surface. Load \`agent-builder-integrations\` whenever the request could -mean that the product is where people invoke or converse with the agent. +For an external product, load \`agent-builder-external-services\` once and +follow it. It covers the chat-integration-versus-callable-tool decision, chat +integration setup, MCP servers, and node tools. - Chat/trigger integration: call \`list_integration_types\`, then \`configure_channel\` with a returned type. Do not call \`resolve_integration\` @@ -24,10 +24,10 @@ mean that the product is where people invoke or converse with the agent. service, call \`resolve_integration\` separately, using \`queries\` as alternative search terms for that one service. Do not infer MCP availability from memory. - - \`kind: "mcp"\`: load \`agent-builder-mcp\` and follow the MCP credential, + - \`kind: "mcp"\`: follow the skill's MCP Servers section — credential, verification, and config workflow. - - \`kind: "node"\`: load \`agent-builder-node-tools\`, use the returned node - results, and continue with \`get_node_types\`. + - \`kind: "node"\`: follow the skill's Node Tools section, use the returned + node results, and continue with \`get_node_types\`. Use \`search_nodes\` directly only when the user explicitly asks for an n8n node, when refining node results, or when a verified MCP server lacks the requested @@ -49,15 +49,15 @@ they cannot perform live network, filesystem, process, timer, or host I/O. #### Node Tools -Load \`agent-builder-node-tools\` after \`resolve_integration\` returns -\`kind: "node"\`, or when the user explicitly requests an n8n node. Follow it -before adding, changing, or removing node-backed tools, \`nodeParameters\`, -\`$fromAI\` usage, or n8n expressions. +Load \`agent-builder-external-services\` when the user explicitly requests an +n8n node, and follow its Node Tools section before adding, changing, or +removing node-backed tools, \`nodeParameters\`, \`$fromAI\` usage, or n8n +expressions. #### MCP Servers -Load \`agent-builder-mcp\` after \`resolve_integration\` returns \`kind: "mcp"\`, or -when the user explicitly requests a custom MCP server. +Load \`agent-builder-external-services\` when the user explicitly requests a +custom MCP server, and follow its MCP Servers section. #### Custom Tools diff --git a/packages/cli/src/modules/agents/builder/resolve-integration.tool.ts b/packages/cli/src/modules/agents/builder/resolve-integration.tool.ts index d229b35a412..88d5cb8d75f 100644 --- a/packages/cli/src/modules/agents/builder/resolve-integration.tool.ts +++ b/packages/cli/src/modules/agents/builder/resolve-integration.tool.ts @@ -22,7 +22,7 @@ const resolveIntegrationInputSchema = z.object({ export function buildResolveIntegrationTool(deps: ResolveIntegrationDeps): BuiltTool { return new Tool(BUILDER_TOOLS.RESOLVE_INTEGRATION) .description( - 'Resolve external services to MCP servers or n8n node tools. Searches the MCP registry first and returns kind: "mcp" when a match exists; only searches agent-eligible n8n node tools and returns kind: "node" when no MCP server matches. For kind: "mcp", load agent-builder-mcp. For kind: "node", load agent-builder-node-tools and use the returned node results.', + 'Resolve external services to MCP servers or n8n node tools. Searches the MCP registry first and returns kind: "mcp" when a match exists; only searches agent-eligible n8n node tools and returns kind: "node" when no MCP server matches. For either kind, load agent-builder-external-services and follow its matching section, using the returned results.', ) .input(resolveIntegrationInputSchema) .handler(async ({ queries }: { queries: string[] }) => { diff --git a/packages/cli/src/modules/agents/builder/skills/__tests__/index.test.ts b/packages/cli/src/modules/agents/builder/skills/__tests__/index.test.ts index f24dc354bda..e82a7c4eeb9 100644 --- a/packages/cli/src/modules/agents/builder/skills/__tests__/index.test.ts +++ b/packages/cli/src/modules/agents/builder/skills/__tests__/index.test.ts @@ -1,9 +1,9 @@ import { getBuilderRuntimeSkills } from '../index'; describe('getBuilderRuntimeSkills', () => { - it('includes the integrations skill', () => { + it('includes the external services skill', () => { const skills = getBuilderRuntimeSkills(); - expect(skills.some((skill) => skill.id === 'agent-builder-integrations')).toBe(true); + expect(skills.some((skill) => skill.id === 'agent-builder-external-services')).toBe(true); }); }); diff --git a/packages/cli/src/modules/agents/builder/skills/external-services.skill.ts b/packages/cli/src/modules/agents/builder/skills/external-services.skill.ts new file mode 100644 index 00000000000..84607154b8b --- /dev/null +++ b/packages/cli/src/modules/agents/builder/skills/external-services.skill.ts @@ -0,0 +1,353 @@ +import type { RuntimeSkill } from '@n8n/agents'; +import { ASK_QUESTIONS_TOOL_NAME, McpServerConfigSchema } from '@n8n/api-types'; +import type { JSONSchema7 } from 'json-schema'; +import { zodToJsonSchema } from 'zod-to-json-schema'; + +import { jsonSchemaToCompactText } from '../../json-config/schema-text-serializer'; +import { INITIAL_BUILD_NOTE } from '../prompts/initial-build.prompt'; + +const mcpServerSchemaText = jsonSchemaToCompactText( + zodToJsonSchema(McpServerConfigSchema) as JSONSchema7, +); + +export function externalServicesSkill(): RuntimeSkill { + return { + id: 'agent-builder-external-services', + name: 'Agent Builder External Services', + description: + 'Use when connecting the target agent to any external product: deciding whether Slack, Linear, Telegram, or another platform is a chat integration/trigger versus an MCP, node, or workflow tool; adding, removing, or updating chat integrations or MCP servers; and wiring n8n node-backed tools (search_nodes/get_node_types discovery, nodeParameters, node credential slots, $fromAI usage, n8n expressions).', + recommendedTools: [ + 'resolve_integration', + 'list_integration_types', + 'configure_channel', + 'search_nodes', + 'get_node_types', + 'ask_credential', + 'verify_mcp_server', + 'read_config', + 'patch_config', + ], + allowedTools: [ + 'resolve_integration', + 'list_integration_types', + 'configure_channel', + 'search_mcp_servers', + 'search_nodes', + 'get_node_types', + 'ask_credential', + 'verify_mcp_server', + 'get_resource_locator_options', + 'ask_questions', + 'read_config', + 'patch_config', + 'write_config', + 'load_skill', + ], + instructions: `\ +## Purpose + +Use this to connect the target agent to external products across all three +surfaces: chat integrations (the \`integrations\` array), MCP servers +(\`mcpServers\`), and n8n node tools (entries in \`tools[]\` with +\`nodeParameters\`). Decide the right surface first, then follow that +section. + +## Integration vs Callable Tool Decision + +Use an integration when the product is the agent's conversation or trigger +surface: humans will mention, message, comment to, or resume the agent there, +or the agent needs to respond in that same platform conversation context. + +Use an MCP, node, or workflow tool when the product is only something the agent +operates on: searching records, creating tickets, updating objects, or sending a +business-process notification while the conversation happens elsewhere. + +Examples: + +- Slack integration: the agent should be chatted with in Slack, respond in + Slack threads, DM users, message channels, add reactions, or render rich UI + to Slack users. +- Linear integration: the agent should be triggered from Linear issues/comments, + understand the current Linear subject, or reply in the same Linear + conversation. +- Linear callable tools: the agent is triggered from Slack, Preview, a task, or a + workflow and only needs to search/create/update Linear tickets via MCP or node + tools. + +For callable (non-chat) services, call \`resolve_integration\` separately per +service and follow the returned \`kind\`: \`"mcp"\` -> MCP Servers section +below, \`"node"\` -> Node Tools section below. + +## Chat Integrations + +The \`integrations\` array controls how the target agent is triggered. + +- These are connected external chat platforms, not built-in Preview chat. +- Call \`list_integration_types\` first. +- Read the returned \`capabilities\`, \`useIntegrationWhen\`, and + \`useNodeToolWhen\` fields before deciding to add an integration. +- Pick one returned \`type\` and pass it to \`configure_channel\` as + \`integrationType\`. ALWAYS use \`configure_channel\` for chat-channel + credentials — never \`ask_credential\` or a raw config write. The setup UI it + shows creates and persists the credential/connection itself; do not follow up + with \`patch_config\`/\`write_config\` to write the credential. +- ${INITIAL_BUILD_NOTE} Instead of \`configure_channel\`: after + \`list_integration_types\` returns the matching type, \`read_config()\` then + \`patch_config\` adding \`{ "type": "", "credentialId": "" }\` + to \`/integrations/-\` (include a minimal valid draft \`settings\` object for + telegram) so the channel appears in the agent panel as needing setup. Pass + the same \`integrationType\` in the trailing \`finish_setup\` call's + \`channels\` array — its card connects or skips the channel itself; if + skipped, list it in the closing setup checklist pointing at the channel + chip in the agent panel. If \`finish_setup\` instead reports the channel as + \`'blocked'\` (the agent could not be published yet), patch in the + credentials/model it collected first; if that resolves every reported + issue, call \`configure_channel\` directly for that channel as a follow-up. + Otherwise leave it for the closing checklist. +- Preserve existing chat integrations unless the user asked to remove them. +- To remove an existing chat integration, call \`read_config\` and inspect + \`config.integrations\`. +- If exactly one existing integration matches the requested platform, remove + that entry with \`patch_config\` by index (or replace \`/integrations\` with a + filtered array when clearer). +- If multiple existing integrations match the requested platform, ask which one + to remove before editing \`integrations\`. +- Removing a chat integration means deleting its entry from + \`integrations[]\`. Do not call \`configure_channel\` to remove a channel. + +### Gotchas + +- Chat integration types must come from \`list_integration_types\`. +- Do not add a chat integration just because the agent needs CRUD or notifications + for that product. Resolve the callable capability through \`resolve_integration\` + unless the product itself is the chat/trigger context. +- For recurring or scheduled runs, create a task (\`create_tasks\`), not an + integration. +- Omitting \`integrations\` from a config write preserves the current channels. + To remove one, write an explicit filtered array or remove the exact array + entry. + +## MCP Servers + +MCP servers expose external tool catalogs to the target agent over HTTP. They +live on the top-level \`mcpServers\` array, and each entry maps 1:1 to a +connected MCP server. Use this section when \`resolve_integration\` returned +\`kind: "mcp"\`, the user explicitly asks to add or edit an MCP server, or the +user provides or asks to configure a custom MCP server. + +### Discovery and setup + +For a generic external-service request, \`resolve_integration\` must select the +integration type before MCP setup. If no resolver result is available yet, +call \`resolve_integration\` with queries matching the requested service. +Resolve one requested service per call; use \`queries\` only for alternative +search terms for that service. + +- If it returns \`kind: "node"\` for a generic service request, follow the Node + Tools section with the returned node results. Stop this MCP workflow. +- If it returns \`kind: "node"\` but the user explicitly requested an MCP server, + do not silently substitute a node tool. Continue with manual MCP setup by + asking for the URL and transport/authentication decision through + \`${ASK_QUESTIONS_TOOL_NAME}\`. +- \`resolve_integration\` returns \`{ kind: "mcp", results: [...] }\` for MCP + matches. Never read server fields from the wrapper; select a result first: + - If \`results[]\` contains one entry, use it as \`selectedResult\`. + - If the request uniquely identifies one entry by \`name\` or \`title\`, use + that entry as \`selectedResult\`. + - If multiple candidates remain, call \`ask_questions\` with the candidate + titles and descriptions; never choose by array order. During an initial + build, do not call \`ask_questions\` for this: pick the best candidate by + title/description relevance yourself, and list the pick as an assumption + in your summary. Use the chosen entry as \`selectedResult\`. If + \`ask_questions\` returns \`{ answered: false }\`, stop MCP setup without + selecting a server, asking for credentials, verifying a connection, or + mutating config. Do not re-present the question. +- Use \`name\`, \`url\`, \`transport\`, \`authentication\`, \`credentialType\`, + \`tools\`, and optional \`metadata\` only from \`selectedResult\`. + +Follow these steps for the selected MCP result: + +1. Credential: call \`ask_credential\` with a short \`purpose\`, using + \`selectedResult.credentialType\` as \`credentialType\`. Never invent + credential IDs. +2. Verify: call \`verify_mcp_server\` with the selected result's \`name\`, \`url\`, + \`transport\`, and \`authentication\`, plus the returned \`credentialId\` as + \`credential\` when authentication is required. +3. Capability check: confirm the verified tool names and descriptions cover the + capability the user requested. +4. Write config: call \`read_config\`, then \`patch_config\` to add the entry to + \`mcpServers[]\` using the patch pattern below. When the entry already + exists and verify returned \`credentialApplied: true\`, skip this step — the + credential is already persisted. + +${INITIAL_BUILD_NOTE} For MCP that means: pick the best candidate as an +assumption (above), then \`read_config()\` and \`patch_config\` a draft +\`/mcpServers/-\` entry using \`name\`, \`url\`, \`transport\`, +\`authentication\`, and \`metadata.nodeTypeName\` from \`selectedResult\` with +\`credential\` omitted, and skip \`verify_mcp_server\` — there is nothing to +authenticate yet. Include the credential in the trailing \`finish_setup\` call; +verify with the returned credential id — on success the tool writes the +credential into the matching entry itself (\`credentialApplied: true\`); no +\`read_config\`/\`patch_config\` follow-up for the credential. Existing-agent +additions keep the immediate ask + verify flow above unchanged. + +If verification succeeds but the tools do not cover the requested capability +for a generic service request, switch to the Node Tools section, call +\`search_nodes\` with the same service queries, and continue with node setup. Do +not add the MCP server merely because its registry entry matched. + +Full schema reference: + +${mcpServerSchemaText} + +### Credential flow + +- For \`bearerAuth\`, call \`ask_credential\` with + \`credentialType: "httpBearerAuth"\`. +- For \`headerAuth\`, call \`ask_credential\` with + \`credentialType: "httpHeaderAuth"\`. +- For \`multipleHeadersAuth\`, call \`ask_credential\` with + \`credentialType: "httpMultipleHeadersAuth"\`. +- For \`mcpOAuth2Api\`, call \`ask_credential\` with + \`credentialType: "mcpOAuth2Api"\`. + +### Testing the connection + +Before writing to config, call \`verify_mcp_server\` with server \`name\`, +\`url\`, \`transport\`, and (if applicable) the credential id from +\`ask_credential\`. + +- Success returns \`{ ok: true, tools: [{ name, description }] }\`, and when a + matching \`mcpServers\` entry exists, also \`credentialApplied: true, + configMutated: true, agentId\` — the credential is written automatically; do + not follow with \`read_config\`/\`patch_config\` for the credential. +- When verify succeeds but \`credentialApplied: false\` and the entry already + exists, fall back to \`read_config\` then \`patch_config\` for the credential. +- Use the returned tool list to populate \`toolFilter.tools\` or + \`approval.tools\` so the user does not need to type tool names manually. +- Failure returns \`{ ok: false, error: "..." }\`. +- If verification fails, explain the error and ask the user to check the URL + or credentials before proceeding. + +### Incomplete setup + +The user can skip the credential prompt, the URL question, or both. Never +invent a credential ID or a placeholder URL to fill the gap, and never abort +the server addition — always persist what is known and let the user finish +setup later: + +- Credential skipped (\`ask_credential\` returned \`{ skipped: true }\`): omit + only the \`credential\` field. +- URL skipped: persist \`url: ""\`. +- Either case: skip \`verify_mcp_server\` (there is nothing to authenticate or + connect to), then \`read_config\` and \`patch_config\` the entry, preserving + every other known field — \`name\`, \`transport\`, \`authentication\`, an + already-selected credential, and registry \`metadata\`. + +### Selecting credentials + +When using a registry-backed server, always use the \`credentialType\` returned +by \`selectedResult\`. + +For custom MCP servers, if credential type is unknown, ask the user which +credential type to use (OAuth2, Bearer Token, Header Auth, Multiple Headers +Auth, or None) via \`${ASK_QUESTIONS_TOOL_NAME}\`. Then map to: + +- \`bearerAuth\` -> \`ask_credential\` with \`credentialType: "httpBearerAuth"\` +- \`headerAuth\` -> \`ask_credential\` with \`credentialType: "httpHeaderAuth"\` +- \`multipleHeadersAuth\` -> \`ask_credential\` with + \`credentialType: "httpMultipleHeadersAuth"\` +- \`mcpOAuth2Api\` -> \`ask_credential\` with \`credentialType: "mcpOAuth2Api"\` + +### Patch pattern + +1. Initialize the array if missing: + \`{ "op": "add", "path": "/mcpServers", "value": [] }\` +2. Append each server: + \`{ "op": "add", "path": "/mcpServers/-", "value": { ... } }\` + +### Gotchas + +- Server \`name\` must be unique across \`mcpServers\` within an agent. +- Never fabricate \`metadata.nodeTypeName\`. +- When \`selectedResult\` includes \`metadata.nodeTypeName\`, include + \`metadata: { nodeTypeName: }\` in the + entry so the UI can render the correct server form. +- A registry match proves server availability, not support for the requested + capability; use the verified live tool list for that decision. + +## Node Tools + +Use this section to discover, configure, and wire node tools into the target +agent's \`tools[]\`, including \`nodeParameters\` and n8n expressions. + +### Workflow + +- For a generic external-service request, call \`resolve_integration\` before + node discovery unless a resolver result is already available. +- If it returns \`kind: "mcp"\`, follow the MCP Servers section instead and stop + this node-tool workflow. +- If it returns \`kind: "node"\`, use its returned node results and call + \`get_node_types\`; do not repeat the same search with \`search_nodes\`. +- Call \`search_nodes\` directly only when the user explicitly requests an n8n + node, when refining node results, or when a verified MCP server lacks the + requested capability. +- Never guess node type names. +- Use the tool node id from discovery, usually ending in \`Tool\`. +- Put fixed values in \`nodeParameters\`; use complete n8n expressions for values the agent should decide at runtime: + \`={{ $fromAI('url', 'The URL to inspect', 'string') }}\`. +- For stable dynamic selectors, load \`agent-builder-resource-locators\` and + follow it. +- Never write literal \`"$fromAI"\` or bare \`$fromAI\`; the node will treat it as the actual value. +- Do not pipe AI-chosen fields through \`$json\`. +- Do not include \`inputSchema\` or \`toolDescription\` for node tools. +- For each required credential slot, call \`ask_credential\` once before the config mutation for an addition to an existing agent. ${INITIAL_BUILD_NOTE} Add the tool with that credential slot omitted; after the trailing \`finish_setup\` resolves the credential, copy the returned credentials into \`node.credentials\` via \`patch_config\`; for resource-locator resolution follow \`agent-builder-resource-locators\` then. Pass the node's credential key as \`credentialSlot\`. On success, copy the returned \`credentials\` object directly to \`node.credentials\`. If skipped, still add the tool and omit only that credential slot. +- When the agent already has a chat channel configured and the tool needs the same + credential type, \`ask_credential\` reuses the channel's credential automatically — + do not ask the user to pick a different one. + +### n8n Expressions + +Node tool parameters inside \`nodeParameters\` can use n8n expressions. +Prefer \`$fromAI\` whenever the target agent should decide a value at runtime. +Do not use \`$fromAI\` for stable resource IDs that the target agent cannot know +at runtime, such as Linear \`teamId\`, project IDs, channel IDs, calendar IDs, +database IDs, table IDs, or other dynamic "Name or ID" selectors. Resolve those +with the \`agent-builder-resource-locators\` skill, \`ask_credential\`, and +\`get_resource_locator_options\`; write the returned \`parameterValue\` into +\`nodeParameters\`. + +- \`={{ /*n8n-auto-generated-fromAI-override*/ $fromAI('fieldName', 'What value to provide', 'string') }}\` +- \`={{ /*n8n-auto-generated-fromAI-override*/ $fromAI('count', 'How many items', 'number') }}\` +- \`={{ /*n8n-auto-generated-fromAI-override*/ $fromAI('enabled', 'Whether to enable this option', 'boolean') }}\` +- \`={{ $now.toISO() }}\` for current date/time. +- \`={{ $today }}\` for the start of today. + +Always wrap expressions in \`={{ }}\`. Never pipe AI-chosen node-tool fields +through \`$json\`; use \`$fromAI\` for those fields instead. + +### Gotchas + +- Do not include \`inputSchema\` or \`toolDescription\` for node tools. +- \`$fromAI(...)\` placeholders define the node tool input schema; do not add it manually. +- Follow \`agent-builder-resource-locators\` for dynamic selector lookup, + credentials, and \`parameterValue\` handling. +- If a required node-tool credential is skipped, add the tool and omit only that credential slot. +- Node tools execute inline, so never use waiting operations such as \`sendAndWait\` + or \`dispatchAndWait\`. When the user requests human approval, configure the + intended non-waiting operation and set \`requireApproval: true\` on the tool. + +## Verify + +- Connected chat integrations were set up through \`configure_channel\`, not + \`ask_credential\` or a manual config write. +- The chosen integration matches \`useIntegrationWhen\`; otherwise resolve the + callable capability through \`resolve_integration\` and use MCP, node, or + workflow tools. +- Generic non-chat external services were routed through \`resolve_integration\` + before MCP or node setup. +- The final \`integrations\` array keeps unrelated integrations intact and + removes only the requested channel entries. +- Node tools use discovered tool node ids and valid node parameters.`, + }; +} diff --git a/packages/cli/src/modules/agents/builder/skills/index.ts b/packages/cli/src/modules/agents/builder/skills/index.ts index 89ed7b688b1..adb11e7fe6e 100644 --- a/packages/cli/src/modules/agents/builder/skills/index.ts +++ b/packages/cli/src/modules/agents/builder/skills/index.ts @@ -1,10 +1,8 @@ import type { RuntimeSkill } from '@n8n/agents'; import { customToolsSkill } from './custom-tools.skill'; -import { integrationsSkill } from './integrations.skill'; -import { mcpSkill } from './mcp.skill'; +import { externalServicesSkill } from './external-services.skill'; import { memorySkill } from './memory.skill'; -import { nodeToolsSkill } from './node-tools.skill'; import { resourceLocatorsSkill } from './resource-locators.skill'; import { subAgentsSkill } from './sub-agents.skill'; import { targetSkillsSkill } from './target-skills.skill'; @@ -13,10 +11,8 @@ import { targetTasksSkill } from './target-tasks.skill'; export function getBuilderRuntimeSkills(): RuntimeSkill[] { return [ customToolsSkill(), - integrationsSkill(), - mcpSkill(), + externalServicesSkill(), memorySkill(), - nodeToolsSkill(), resourceLocatorsSkill(), subAgentsSkill(), targetSkillsSkill(), diff --git a/packages/cli/src/modules/agents/builder/skills/integrations.skill.ts b/packages/cli/src/modules/agents/builder/skills/integrations.skill.ts deleted file mode 100644 index 4365f5b6f07..00000000000 --- a/packages/cli/src/modules/agents/builder/skills/integrations.skill.ts +++ /dev/null @@ -1,127 +0,0 @@ -import type { RuntimeSkill } from '@n8n/agents'; - -export function integrationsSkill(): RuntimeSkill { - return { - id: 'agent-builder-integrations', - name: 'Agent Builder Integrations', - description: - 'Use when deciding whether Slack, Linear, Telegram, or another external platform should be a target-agent chat integration/trigger versus an MCP, node, or workflow tool, and when adding, changing, or removing chat integrations; not for built-in Build chat or Preview chat behavior.', - recommendedTools: [ - 'resolve_integration', - 'list_integration_types', - 'configure_channel', - 'ask_questions', - 'read_config', - 'patch_config', - ], - allowedTools: [ - 'resolve_integration', - 'list_integration_types', - 'configure_channel', - 'ask_questions', - 'read_config', - 'patch_config', - 'write_config', - 'load_skill', - ], - instructions: `\ -## Purpose - -Use this to decide whether the target agent needs an entry in \`integrations\` -or an MCP, node, or workflow tool for an external product, then configure -\`integrations\` only when the integration is the right surface. - -## Use when - -- The user asks to add, update, or remove entries in the target agent's - \`integrations\` array. -- The user asks to connect the target agent to an external chat platform with - credentials. - -## Integration vs Callable Tool Decision - -Use an integration when the product is the agent's conversation or trigger -surface: humans will mention, message, comment to, or resume the agent there, -or the agent needs to respond in that same platform conversation context. - -Use an MCP, node, or workflow tool when the product is only something the agent -operates on: searching records, creating tickets, updating objects, or sending a -business-process notification while the conversation happens elsewhere. - -Examples: - -- Slack integration: the agent should be chatted with in Slack, respond in - Slack threads, DM users, message channels, add reactions, or render rich UI - to Slack users. -- Linear integration: the agent should be triggered from Linear issues/comments, - understand the current Linear subject, or reply in the same Linear - conversation. -- Linear callable tools: the agent is triggered from Slack, Preview, a task, or a - workflow and only needs to search/create/update Linear tickets via MCP or node - tools. - -## Workflow - -The \`integrations\` array controls how the target agent is triggered. - -### Chat Integrations - -- These are connected external chat platforms, not built-in Preview chat. -- Call \`list_integration_types\` first. -- Read the returned \`capabilities\`, \`useIntegrationWhen\`, and - \`useNodeToolWhen\` fields before deciding to add an integration. -- Pick one returned \`type\` and pass it to \`configure_channel\` as - \`integrationType\`. ALWAYS use \`configure_channel\` for chat-channel - credentials — never \`ask_credential\` or a raw config write. The setup UI it - shows creates and persists the credential/connection itself; do not follow up - with \`patch_config\`/\`write_config\` to write the credential. -- Preserve existing chat integrations unless the user asked to remove them. -- To remove an existing chat integration, call \`read_config\` and inspect - \`config.integrations\`. -- If exactly one existing integration matches the requested platform, remove - that entry with \`patch_config\` by index (or replace \`/integrations\` with a - filtered array when clearer). -- If multiple existing integrations match the requested platform, ask which one - to remove before editing \`integrations\`. -- Removing a chat integration means deleting its entry from - \`integrations[]\`. Do not call \`configure_channel\` to remove a channel. - -### Callable External Services - -When the product is not a chat/trigger integration, call \`resolve_integration\` -with queries matching the requested service before loading MCP or node-tool -skills. - -- If it returns \`kind: "mcp"\`, load \`agent-builder-mcp\` and follow the MCP - credential, verification, and config workflow. -- If it returns \`kind: "node"\`, load \`agent-builder-node-tools\`, use the - returned node results with \`get_node_types\`, and ask for every required - credential. -- Use workflow tools when the capability should come from an existing workflow - instead of a direct MCP or node tool. - -## Gotchas - -- Chat integration types must come from \`list_integration_types\`. -- Do not add a chat integration just because the agent needs CRUD or notifications - for that product. Resolve the callable capability through \`resolve_integration\` - unless the product itself is the chat/trigger context. -- For recurring or scheduled runs, create a task (\`create_tasks\`), not an - integration. -- Omitting \`integrations\` from a config write preserves the current channels. - To remove one, write an explicit filtered array or remove the exact array - entry. - -## Verify - -- Connected chat integrations were set up through \`configure_channel\`, not - \`ask_credential\` or a manual config write. -- The chosen integration matches \`useIntegrationWhen\`; otherwise resolve the - callable capability through \`resolve_integration\` and use MCP, node, or - workflow tools. -- Generic non-chat external services were routed through \`resolve_integration\` - before MCP or node setup. -- The final \`integrations\` array keeps unrelated integrations intact and - removes only the requested channel entries.`, - }; -} diff --git a/packages/cli/src/modules/agents/builder/skills/mcp.skill.ts b/packages/cli/src/modules/agents/builder/skills/mcp.skill.ts deleted file mode 100644 index aaee3027a73..00000000000 --- a/packages/cli/src/modules/agents/builder/skills/mcp.skill.ts +++ /dev/null @@ -1,176 +0,0 @@ -import type { RuntimeSkill } from '@n8n/agents'; -import { ASK_QUESTIONS_TOOL_NAME, McpServerConfigSchema } from '@n8n/api-types'; -import type { JSONSchema7 } from 'json-schema'; -import { zodToJsonSchema } from 'zod-to-json-schema'; - -import { jsonSchemaToCompactText } from '../../json-config/schema-text-serializer'; - -const mcpServerSchemaText = jsonSchemaToCompactText( - zodToJsonSchema(McpServerConfigSchema) as JSONSchema7, -); - -export function mcpSkill(): RuntimeSkill { - return { - id: 'agent-builder-mcp', - name: 'Agent builder MCP servers', - description: - 'Use when adding, removing, or updating MCP (Model Context Protocol) servers on the target agent.', - recommendedTools: [ - 'resolve_integration', - 'ask_questions', - 'ask_credential', - 'verify_mcp_server', - 'read_config', - 'patch_config', - ], - allowedTools: [ - 'resolve_integration', - 'search_mcp_servers', - 'search_nodes', - 'get_node_types', - 'ask_credential', - 'verify_mcp_server', - 'ask_questions', - 'read_config', - 'patch_config', - 'write_config', - 'load_skill', - ], - instructions: `\ -## Purpose - -Use this to manage external MCP server connections in the target agent config. -MCP servers expose external tool catalogs to the target agent over HTTP. They -live on the top-level \`mcpServers\` array, and each entry maps 1:1 to a -connected MCP server. - -## Use when: - -- \`resolve_integration\` returned \`kind: "mcp"\`. -- The user explicitly asks to add or edit an MCP server. -- The user provides or asks to configure a custom MCP server. - -## Workflow - -### Discovery and setup - -For a generic external-service request, \`resolve_integration\` must select the -integration type before this skill is loaded. If no resolver result is -available yet, call \`resolve_integration\` with queries matching the requested -service. Resolve one requested service per call; use \`queries\` only for -alternative search terms for that service. - -- If it returns \`kind: "node"\` for a generic service request, load - \`agent-builder-node-tools\` and continue with the returned node results. Stop - this MCP workflow. -- If it returns \`kind: "node"\` but the user explicitly requested an MCP server, - do not silently substitute a node tool. Continue with manual MCP setup by - asking for the URL and transport/authentication decision through - \`${ASK_QUESTIONS_TOOL_NAME}\`. -- \`resolve_integration\` returns \`{ kind: "mcp", results: [...] }\` for MCP - matches. Never read server fields from the wrapper; select a result first: - - If \`results[]\` contains one entry, use it as \`selectedResult\`. - - If the request uniquely identifies one entry by \`name\` or \`title\`, use - that entry as \`selectedResult\`. - - If multiple candidates remain, call \`ask_questions\` with the candidate - titles and descriptions; never choose by array order. Use the chosen entry - as \`selectedResult\`. If \`ask_questions\` returns \`{ answered: false }\`, - stop MCP setup without selecting a server, asking for credentials, verifying - a connection, or mutating config. Do not re-present the question. -- Use \`name\`, \`url\`, \`transport\`, \`authentication\`, \`credentialType\`, - \`tools\`, and optional \`metadata\` only from \`selectedResult\`. - -Follow these steps for the selected MCP result: - -1. Credential: call \`ask_credential\` with a short \`purpose\`, using - \`selectedResult.credentialType\` as \`credentialType\`. Never invent - credential IDs. -2. Verify: call \`verify_mcp_server\` with the selected result's \`name\`, \`url\`, - \`transport\`, and \`authentication\`, plus the returned \`credentialId\` as - \`credential\` when authentication is required. -3. Capability check: confirm the verified tool names and descriptions cover the - capability the user requested. -4. Write config: call \`read_config\`, then \`patch_config\` to add the entry to - \`mcpServers[]\` using the patch pattern below. - -If verification succeeds but the tools do not cover the requested capability -for a generic service request, load \`agent-builder-node-tools\`, call -\`search_nodes\` with the same service queries, and continue with node setup. Do -not add the MCP server merely because its registry entry matched. - -Full schema reference: - -${mcpServerSchemaText} - -### Credential flow - -- For \`bearerAuth\`, call \`ask_credential\` with - \`credentialType: "httpBearerAuth"\`. -- For \`headerAuth\`, call \`ask_credential\` with - \`credentialType: "httpHeaderAuth"\`. -- For \`multipleHeadersAuth\`, call \`ask_credential\` with - \`credentialType: "httpMultipleHeadersAuth"\`. -- For \`mcpOAuth2Api\`, call \`ask_credential\` with - \`credentialType: "mcpOAuth2Api"\`. - -### Testing the connection - -Before writing to config, call \`verify_mcp_server\` with server \`name\`, -\`url\`, \`transport\`, and (if applicable) the credential id from -\`ask_credential\`. - -- Success returns \`{ ok: true, tools: [{ name, description }] }\`. -- Use the returned tool list to populate \`toolFilter.tools\` or - \`approval.tools\` so the user does not need to type tool names manually. -- Failure returns \`{ ok: false, error: "..." }\`. -- If verification fails, explain the error and ask the user to check the URL - or credentials before proceeding. - -### Incomplete setup - -The user can skip the credential prompt, the URL question, or both. Never -invent a credential ID or a placeholder URL to fill the gap, and never abort -the server addition — always persist what is known and let the user finish -setup later: - -- Credential skipped (\`ask_credential\` returned \`{ skipped: true }\`): omit - only the \`credential\` field. -- URL skipped: persist \`url: ""\`. -- Either case: skip \`verify_mcp_server\` (there is nothing to authenticate or - connect to), then \`read_config\` and \`patch_config\` the entry, preserving - every other known field — \`name\`, \`transport\`, \`authentication\`, an - already-selected credential, and registry \`metadata\`. - -### Selecting credentials - -When using a registry-backed server, always use the \`credentialType\` returned -by \`selectedResult\`. - -For custom MCP servers, if credential type is unknown, ask the user which -credential type to use (OAuth2, Bearer Token, Header Auth, Multiple Headers -Auth, or None) via \`${ASK_QUESTIONS_TOOL_NAME}\`. Then map to: - -- \`bearerAuth\` -> \`ask_credential\` with \`credentialType: "httpBearerAuth"\` -- \`headerAuth\` -> \`ask_credential\` with \`credentialType: "httpHeaderAuth"\` -- \`multipleHeadersAuth\` -> \`ask_credential\` with - \`credentialType: "httpMultipleHeadersAuth"\` -- \`mcpOAuth2Api\` -> \`ask_credential\` with \`credentialType: "mcpOAuth2Api"\` - -### Patch pattern - -1. Initialize the array if missing: - \`{ "op": "add", "path": "/mcpServers", "value": [] }\` -2. Append each server: - \`{ "op": "add", "path": "/mcpServers/-", "value": { ... } }\` - -## Gotchas - -- Server \`name\` must be unique across \`mcpServers\` within an agent. -- Never fabricate \`metadata.nodeTypeName\`. -- When \`selectedResult\` includes \`metadata.nodeTypeName\`, include - \`metadata: { nodeTypeName: }\` in the - entry so the UI can render the correct server form. -- A registry match proves server availability, not support for the requested - capability; use the verified live tool list for that decision.`, - }; -} diff --git a/packages/cli/src/modules/agents/builder/skills/memory.skill.ts b/packages/cli/src/modules/agents/builder/skills/memory.skill.ts index 2da43de9e64..2ec4eb755ff 100644 --- a/packages/cli/src/modules/agents/builder/skills/memory.skill.ts +++ b/packages/cli/src/modules/agents/builder/skills/memory.skill.ts @@ -1,5 +1,7 @@ import type { RuntimeSkill } from '@n8n/agents'; +import { INITIAL_BUILD_NOTE } from '../prompts/initial-build.prompt'; + export function memorySkill(): RuntimeSkill { return { id: 'agent-builder-memory', @@ -47,7 +49,7 @@ Memory (long-term, cross-session recall). ## Episodic Memory - Enable \`memory.episodicMemory\` only when the user asks for Episodic Memory, long-term memory, prior conversations, remembered decisions, exact artifacts, or cross-session memory. -- Before enabling it, call \`ask_embedding_credential({ credentialType: "openAiApi", purpose: "OpenAI credential for Episodic Memory embeddings" })\`. +- Before enabling it, call \`ask_embedding_credential({ credentialType: "openAiApi", purpose: "OpenAI credential for Episodic Memory embeddings" })\`. Without the assistant proxy: ${INITIAL_BUILD_NOTE} Leave Episodic Memory disabled, include an \`openAiApi\` entry in the trailing \`finish_setup\` call, and enable it with the returned credential id — the draft schema requires a credential to enable it. - On success, set \`memory.episodicMemory = { "enabled": true, "credential": "" }\`, using the returned \`credentialId\` value. This can be a real credential id or \`"managed"\` when the assistant proxy is available. Preserve existing \`topK\` or \`maxEntriesPerRun\`. - \`memory.episodicMemory.credential\` is only for OpenAI embeddings. It is separate from optional \`extractorModel\` and \`reflectorModel\` worker credentials. - If credential selection is skipped, do not enable Episodic Memory; explain that it needs an OpenAI credential for embeddings. diff --git a/packages/cli/src/modules/agents/builder/skills/node-tools.skill.ts b/packages/cli/src/modules/agents/builder/skills/node-tools.skill.ts deleted file mode 100644 index 5938b36129e..00000000000 --- a/packages/cli/src/modules/agents/builder/skills/node-tools.skill.ts +++ /dev/null @@ -1,95 +0,0 @@ -import type { RuntimeSkill } from '@n8n/agents'; - -export function nodeToolsSkill(): RuntimeSkill { - return { - id: 'agent-builder-node-tools', - name: 'Agent Builder Node Tools', - description: - 'Use when resolve_integration returns kind: "node" or the user explicitly requests an n8n node-backed tool: search_nodes/get_node_types discovery, nodeParameters, node credential slots, $fromAI usage, or other n8n expressions.', - recommendedTools: [ - 'resolve_integration', - 'search_nodes', - 'get_node_types', - 'ask_credential', - 'read_config', - 'patch_config', - ], - allowedTools: [ - 'resolve_integration', - 'search_nodes', - 'get_node_types', - 'ask_credential', - 'get_resource_locator_options', - 'ask_questions', - 'read_config', - 'patch_config', - 'write_config', - 'load_skill', - ], - instructions: `\ -## Purpose - -Use this to discover, configure, and wire node tools into the target agent's -\`tools[]\`, including \`nodeParameters\` and n8n expressions. - -## Workflow - -- For a generic external-service request, call \`resolve_integration\` before - node discovery unless a resolver result is already available. -- If it returns \`kind: "mcp"\`, load \`agent-builder-mcp\` and stop this node-tool - workflow. -- If it returns \`kind: "node"\`, use its returned node results and call - \`get_node_types\`; do not repeat the same search with \`search_nodes\`. -- Call \`search_nodes\` directly only when the user explicitly requests an n8n - node, when refining node results, or when a verified MCP server lacks the - requested capability. -- Never guess node type names. -- Use the tool node id from discovery, usually ending in \`Tool\`. -- Put fixed values in \`nodeParameters\`; use complete n8n expressions for values the agent should decide at runtime: - \`={{ $fromAI('url', 'The URL to inspect', 'string') }}\`. -- For stable dynamic selectors, load \`agent-builder-resource-locators\` and - follow it. -- Never write literal \`"$fromAI"\` or bare \`$fromAI\`; the node will treat it as the actual value. -- Do not pipe AI-chosen fields through \`$json\`. -- Do not include \`inputSchema\` or \`toolDescription\` for node tools. -- For each required credential slot, call \`ask_credential\` once before config mutation. Pass the node's credential key as \`credentialSlot\`. On success, copy the returned \`credentials\` object directly to \`node.credentials\`. If skipped, still add the tool and omit only that credential slot. -- When the agent already has a chat channel configured and the tool needs the same - credential type, \`ask_credential\` reuses the channel's credential automatically — - do not ask the user to pick a different one. - -## n8n Expressions - -Node tool parameters inside \`nodeParameters\` can use n8n expressions. -Prefer \`$fromAI\` whenever the target agent should decide a value at runtime. -Do not use \`$fromAI\` for stable resource IDs that the target agent cannot know -at runtime, such as Linear \`teamId\`, project IDs, channel IDs, calendar IDs, -database IDs, table IDs, or other dynamic "Name or ID" selectors. Resolve those -with the \`agent-builder-resource-locators\` skill, \`ask_credential\`, and -\`get_resource_locator_options\`; write the returned \`parameterValue\` into -\`nodeParameters\`. - -- \`={{ /*n8n-auto-generated-fromAI-override*/ $fromAI('fieldName', 'What value to provide', 'string') }}\` -- \`={{ /*n8n-auto-generated-fromAI-override*/ $fromAI('count', 'How many items', 'number') }}\` -- \`={{ /*n8n-auto-generated-fromAI-override*/ $fromAI('enabled', 'Whether to enable this option', 'boolean') }}\` -- \`={{ $now.toISO() }}\` for current date/time. -- \`={{ $today }}\` for the start of today. - -Always wrap expressions in \`={{ }}\`. Never pipe AI-chosen node-tool fields -through \`$json\`; use \`$fromAI\` for those fields instead. - -## Gotchas - -- Do not include \`inputSchema\` or \`toolDescription\` for node tools. -- \`$fromAI(...)\` placeholders define the node tool input schema; do not add it manually. -- Follow \`agent-builder-resource-locators\` for dynamic selector lookup, - credentials, and \`parameterValue\` handling. -- If a required node-tool credential is skipped, add the tool and omit only that credential slot. -- Node tools execute inline, so never use waiting operations such as \`sendAndWait\` - or \`dispatchAndWait\`. When the user requests human approval, configure the - intended non-waiting operation and set \`requireApproval: true\` on the tool. - -## Verify - -- Node tools use discovered tool node ids and valid node parameters.`, - }; -} diff --git a/packages/cli/src/modules/agents/builder/skills/resource-locators.skill.ts b/packages/cli/src/modules/agents/builder/skills/resource-locators.skill.ts index 3c40ead555c..9472a12c744 100644 --- a/packages/cli/src/modules/agents/builder/skills/resource-locators.skill.ts +++ b/packages/cli/src/modules/agents/builder/skills/resource-locators.skill.ts @@ -1,5 +1,7 @@ import type { RuntimeSkill } from '@n8n/agents'; +import { INITIAL_BUILD_NOTE } from '../prompts/initial-build.prompt'; + export function resourceLocatorsSkill(): RuntimeSkill { return { id: 'agent-builder-resource-locators', @@ -41,6 +43,18 @@ locator values that the target agent cannot reliably guess at runtime. - \`write_config\` or \`patch_config\` rejects a node parameter with a dynamic selector / \`get_resource_locator_options\` error. +## Initial-build timing + +${INITIAL_BUILD_NOTE} If the tool needs a credential to resolve a stable +selector, skip adding that tool for now instead of blocking the rest of the +build. Include the credential in the trailing \`finish_setup\` call, then run +the option lookup with the returned credential in the same turn and write the +config mutation only when the result is unambiguous (an exact or single +filtered match). If options remain ambiguous, do not call \`ask_questions\` — +leave the tool deferred and add a one-line setup checklist item naming the +pending selection; resolve it in a later turn. In an addition to an existing +agent, resolve the credential and any ambiguity immediately instead. + ## Workflow 1. Discover and inspect the node with \`search_nodes\`, then \`get_node_types\`. @@ -62,9 +76,10 @@ locator values that the target agent cannot reliably guess at runtime. - current \`nodeParameters\` - returned \`credentials\`, when available - \`filter\` when the user named a specific team, channel, project, or object -6. If results are ambiguous, use \`ask_questions\` with the returned option names. - If there are many pages, retry with \`paginationToken\` or a narrower - \`filter\`. +6. If results are ambiguous, use \`ask_questions\` with the returned option + names (existing agents only — during an initial build, defer per + Initial-build timing above). If there are many pages, retry with + \`paginationToken\` or a narrower \`filter\`. 7. Write the selected result's \`parameterValue\` exactly into \`nodeParameters\`. For resource locators this is an object with \`__rl\`, \`mode\`, and \`value\`; for classic dynamic options this is the raw ID/value. diff --git a/packages/cli/src/modules/agents/builder/skills/target-skills.skill.ts b/packages/cli/src/modules/agents/builder/skills/target-skills.skill.ts index 6ba0d1d5f55..b2fbdb3f1e0 100644 --- a/packages/cli/src/modules/agents/builder/skills/target-skills.skill.ts +++ b/packages/cli/src/modules/agents/builder/skills/target-skills.skill.ts @@ -43,21 +43,23 @@ ${SKILL_BODY_FORMAT_RULE} ${SKILL_BODY_TEMPLATE} -## Ask first (required) +## Fill the template with assumptions (required) Do NOT call \`create_skills\` until you have enough concrete domain detail to write a genuinely useful skill: a specific routing description and a body whose applicable sections are filled with real content (the actual steps, rules, -examples, and edge cases). If any of that is missing, ask the user clarifying -questions (use \`ask_questions\`, batching multiple questions into one call — -discrete options for choices, or \`type: "text"\` for open-ended) until you can -write it. Never create a placeholder or vague skill. +examples, and edge cases). Derive missing domain detail from the user's +stated goal as stated assumptions, and list them in your summary. Use +\`ask_questions\` only when even a reasonable assumption is impossible — never +during an initial build: mark the task \`blocked\` instead, per the Initial +Build rules in your system prompt. Never create a placeholder or vague +skill. ## Workflow -- Gather the domain detail you need, asking clarifying questions until the - description and every applicable body section can be written with concrete - content, for every skill you plan to create. +- Fill the domain detail you need, deriving missing detail from the goal as + stated assumptions so the description and every applicable body section can + be written with concrete content, for every skill you plan to create. - Write each skill's \`description\` as the routing contract and \`instructions\` using the template above. Put all "when to use" / "when not to use" guidance in the description, never in the body (the body is invisible until the skill @@ -104,7 +106,8 @@ write it. Never create a placeholder or vague skill. - \`create_skills\` does not attach any skill to the target agent config. - A skill that is useful for every request probably belongs in instructions, not in \`skills\`. - A vague description creates a vague skill, even if the body is excellent. -- Do not create placeholder or vague skills; ask for missing domain details first. +- Do not create placeholder or vague skills; derive missing domain details from + the stated goal as assumptions instead. - Do not call \`create_skills\` once per skill when several are ready — batch them into one call so the whole set is stored in a single round trip. diff --git a/packages/cli/src/modules/agents/builder/skills/target-tasks.skill.ts b/packages/cli/src/modules/agents/builder/skills/target-tasks.skill.ts index 8172d6c33c5..1287a38404c 100644 --- a/packages/cli/src/modules/agents/builder/skills/target-tasks.skill.ts +++ b/packages/cli/src/modules/agents/builder/skills/target-tasks.skill.ts @@ -53,26 +53,30 @@ ${TASK_OBJECTIVE_FORMAT_RULE} ${TASK_OBJECTIVE_TEMPLATE} -## Ask first (required) +## Fill the template with assumptions (required) Do NOT call \`create_tasks\` for a task until BOTH of these are true for it: 1. You can fill EVERY section of the objective template above with concrete, - specific content — no placeholders, no guesses, nothing left to "refine + specific content — no placeholders, nothing left to "refine later". The objective is the ONLY message the agent receives when the task fires, so it must stand on its own and must not rely on the current chat. -2. The schedule is concrete — how often and at what time it should run. + When the user did not specify a detail, derive it from the goal as a + stated assumption and list it in your summary. +2. The schedule is concrete — how often and at what time it should run. If + the user did not specify a cadence, pick a sensible default and state it + as an assumption. -If any section would be empty or a guess, ask the user clarifying questions (use -\`ask_questions\`, batching multiple questions into one call — discrete options for -choices, or \`type: "text"\` for open-ended) until you can complete the whole -template and pin down the cadence for every task. Never create a placeholder or +Use \`ask_questions\` only when even a reasonable assumption is impossible — +never during an initial build: mark the task \`blocked\` instead, per the +Initial Build rules in your system prompt. Never create a placeholder or "refine-it-later" task. ## Workflow -- Gather everything the template needs (every objective section + the cadence) - for every task, asking clarifying questions until no section is a guess. +- Fill everything the template needs (every objective section + the cadence) + for every task, deriving missing details from the goal as stated + assumptions instead of asking. - Write each objective using the exact template above, filling each section. - Make sure the agent already has every tool the steps need (an integration, node/workflow tool, or web search). If something is missing, add it to the agent diff --git a/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts b/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts index 8b7740fb86c..c7be30c2e4c 100644 --- a/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts +++ b/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts @@ -10,10 +10,17 @@ import { BUILDER_TOOLS } from './builder-tool-names'; import { buildMcpClientForServer } from '../json-config/mcp-client-factory'; export interface VerifyMcpServerDeps { + agentId?: string; credentialProvider: CredentialProvider; oauthService: OauthService; projectId: string; proxyFetch: CustomFetch; + /** When verification succeeds with a credential, writes it into the matching + * mcpServers entry so the builder can skip read_config → patch_config. */ + applyCredentialToMcpServer?: ( + serverName: string, + credentialId: string, + ) => Promise<{ applied: boolean }>; } /** Default deadline for the whole verify operation (connect + listTools) when the @@ -108,7 +115,10 @@ export function buildVerifyMcpServerTool(deps: VerifyMcpServerDeps): BuiltTool { 'Establishes a temporary connection, lists the available tools, then closes the connection. ' + 'Returns { ok: true, tools: [{ name, description }] } on success, or ' + '{ ok: false, error: string } on failure. ' + - 'Call this after ask_credential (when authentication is not "none") and before patch_config.', + 'When a credential is provided and a matching mcpServers entry already exists, ' + + 'a successful verify also writes the credential into that entry ' + + '({ credentialApplied: true, configMutated: true, agentId }) — no read_config/patch_config follow-up. ' + + 'Call this after ask_credential when authentication is not "none".', ) .input(verifyMcpServerInputSchema) .handler(async (input: VerifyMcpServerInput, ctx: ToolContext) => { @@ -127,12 +137,35 @@ export function buildVerifyMcpServerTool(deps: VerifyMcpServerDeps): BuiltTool { deps, ); const tools = await listToolsWithinDeadline(client, timeoutMs, ctx.abortSignal); + const mappedTools = tools.map((t) => ({ + name: t.name, + description: t.description ?? '', + })); + + if (input.credential && deps.applyCredentialToMcpServer) { + try { + const { applied } = await deps.applyCredentialToMcpServer(input.name, input.credential); + if (applied && deps.agentId) { + return { + ok: true, + tools: mappedTools, + credentialApplied: true, + configMutated: true, + agentId: deps.agentId, + }; + } + } catch { + return { + ok: true, + tools: mappedTools, + credentialApplied: false, + }; + } + } + return { ok: true, - tools: tools.map((t) => ({ - name: t.name, - description: t.description ?? '', - })), + tools: mappedTools, }; } catch (error) { return { diff --git a/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts b/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts index 74034fdb6dd..1582f3114f5 100644 --- a/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts +++ b/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts @@ -368,7 +368,7 @@ describe('SlackAppSetupService', () => { 'project-1', user, undefined, - { syncIntegrations: false }, + { syncIntegrations: false, ignoreDraftIntegrations: true }, ); expect(chatIntegrationService.broadcastIntegrationChange).toHaveBeenCalledWith( 'agent-1', @@ -429,7 +429,7 @@ describe('SlackAppSetupService', () => { 'project-1', user, undefined, - { syncIntegrations: false }, + { syncIntegrations: false, ignoreDraftIntegrations: true }, ); expect( agentIntegrationPersistenceService.saveCredentialIntegration.mock.invocationCallOrder[0], diff --git a/packages/cli/src/modules/agents/integrations/slack-app-setup.service.ts b/packages/cli/src/modules/agents/integrations/slack-app-setup.service.ts index 7b64d475018..ebd6112e33b 100644 --- a/packages/cli/src/modules/agents/integrations/slack-app-setup.service.ts +++ b/packages/cli/src/modules/agents/integrations/slack-app-setup.service.ts @@ -266,6 +266,7 @@ export class SlackAppSetupService { undefined, { syncIntegrations: false, + ignoreDraftIntegrations: true, }, ); await this.chatIntegrationService.connect(session.agentId, integration, session.projectId); diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.service.test.ts index 65e79ee96e0..f8198907026 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.service.test.ts @@ -2394,6 +2394,45 @@ describe('InstanceAiService — suspended run user revalidation', () => { ); }); + it('rebinds the suspended orchestration context tracing to the resume trace', async () => { + const service = createSuspendedRunResumeService(); + const freshUser = { id: 'user-1', disabled: false } as User; + service.revalidateActiveUser.mockResolvedValue(freshUser); + const staleTracing = { id: 'stale-trace' }; + const resumeTracing = { id: 'resume-trace' }; + const orchestrationContext = { tracing: staleTracing }; + service.runState.findSuspendedByRequestId.mockReturnValue({ + agent: {}, + runId: 'run-1', + agentRunId: 'agent-run-1', + threadId: 'thread-a', + user: fakeUser, + toolCallId: 'tool-call-1', + toolName: 'workflows', + suspendPayload: { workflowId: 'wf-1', setupRequests: [] }, + abortController: new AbortController(), + tracing: staleTracing, + modelId: undefined, + messageGroupId: 'group-1', + checkpoint: undefined, + runHandoff: undefined, + orchestrationContext, + }); + service.tracing.createOrchestratorResumeTraceContext.mockResolvedValue(resumeTracing); + + await service.resumeSuspendedRun('user-1', 'req-1', { approved: true }); + + expect(orchestrationContext.tracing).toBe(resumeTracing); + expect(service.processResumedStream).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ approved: true }), + expect.objectContaining({ + orchestrationContext, + tracing: resumeTracing, + }), + ); + }); + it('rebuilds the agent when autoSetup is set, and resumes with the rebuilt one', async () => { const service = createSuspendedRunResumeService(); const freshUser = { id: 'user-1', disabled: false } as User; @@ -2532,6 +2571,7 @@ describe('InstanceAiService — rebuildAgentForAutoSetupResume', () => { expect(result).toEqual({ agent: rebuiltAgent, modelId: { provider: 'anthropic', model: 'claude' }, + orchestrationContext, }); expect(createOrchestratorRunControl).toHaveBeenCalledWith(orchestrationContext, runHandoff); }); diff --git a/packages/cli/src/modules/instance-ai/instance-ai.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.service.ts index b0c35e064d3..eb715f9d4a4 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.service.ts @@ -3684,6 +3684,7 @@ export class InstanceAiService { runId, agentRunId: result.agentRunId, agent, + orchestrationContext, threadId, user, toolCallId: result.suspension.toolCallId, @@ -4526,6 +4527,7 @@ export class InstanceAiService { runId: orphan.runId, agentRunId: orphan.checkpointKey, agent, + orchestrationContext: environment.orchestrationContext, threadId: orphan.threadId, user, toolCallId: orphan.toolCallId, @@ -4584,7 +4586,12 @@ export class InstanceAiService { runHandoff: OrchestratorRunHandoffState | undefined, messageGroupId?: string, ): Promise< - { agent: Awaited>; modelId: ModelConfig } | undefined + | { + agent: Awaited>; + modelId: ModelConfig; + orchestrationContext: OrchestrationContext; + } + | undefined > { try { const rebuilt = await this.buildFreshInstanceAgent( @@ -4597,7 +4604,11 @@ export class InstanceAiService { this.threadPushRef.get(threadId), ); createOrchestratorRunControl(rebuilt.orchestrationContext, runHandoff ?? {}); - return { agent: rebuilt.agent, modelId: rebuilt.modelId }; + return { + agent: rebuilt.agent, + modelId: rebuilt.modelId, + orchestrationContext: rebuilt.orchestrationContext, + }; } catch (error: unknown) { this.logger.warn('Failed to rebuild agent for credential auto-setup resume', { threadId, @@ -4638,6 +4649,7 @@ export class InstanceAiService { checkpoint, plannedBuild, runHandoff, + orchestrationContext, } = suspended; if (user.id !== requestingUserId) return null; @@ -4711,8 +4723,16 @@ export class InstanceAiService { }); const effectiveTracing = resumeTracing ?? tracing; + // Orchestration tools (e.g. build-agent) read `context.tracing` at call + // time from this shared object; without the rebind the resumed sub-agent + // emits spans through the suspended turn's shut-down trace runtime. + if (orchestrationContext && effectiveTracing) { + orchestrationContext.tracing = effectiveTracing; + } + let resumeAgent = agent; let resumeModelId = modelId; + let resumeOrchestrationContext = orchestrationContext; if (data.autoSetup) { const rebuilt = await this.rebuildAgentForAutoSetupResume( activeUser, @@ -4729,6 +4749,7 @@ export class InstanceAiService { } resumeAgent = rebuilt.agent; resumeModelId = rebuilt.modelId; + resumeOrchestrationContext = rebuilt.orchestrationContext; } this.startProcessResumedStream(resumeAgent, resumeData, { @@ -4743,6 +4764,7 @@ export class InstanceAiService { abortController, snapshotStorage: this.dbSnapshotStorage, tracing: effectiveTracing, + orchestrationContext: resumeOrchestrationContext, modelId: resumeModelId, checkpoint, plannedBuild, @@ -4772,6 +4794,7 @@ export class InstanceAiService { abortController: AbortController; snapshotStorage: DbSnapshotStorage; tracing?: InstanceAiTraceContext; + orchestrationContext?: OrchestrationContext; modelId?: ModelConfig; checkpoint?: { isCheckpointFollowUp: true; checkpointTaskId: string }; plannedBuild?: PlannedBuildFollowUp; @@ -4854,6 +4877,7 @@ export class InstanceAiService { runId: opts.runId, agentRunId: result.agentRunId, agent, + orchestrationContext: opts.orchestrationContext, threadId: opts.threadId, user: opts.user, toolCallId: result.suspension.toolCallId, diff --git a/packages/cli/src/services/__tests__/free-ai-credits.service.test.ts b/packages/cli/src/services/__tests__/free-ai-credits.service.test.ts new file mode 100644 index 00000000000..f146694b065 --- /dev/null +++ b/packages/cli/src/services/__tests__/free-ai-credits.service.test.ts @@ -0,0 +1,102 @@ +import type { LicenseState } from '@n8n/backend-common'; +import type { GlobalConfig } from '@n8n/config'; +import type { User } from '@n8n/db'; +import { mock } from 'vitest-mock-extended'; + +import { FREE_AI_CREDITS_CREDENTIAL_NAME } from '@/constants'; +import type { CredentialsService } from '@/credentials/credentials.service'; +import type { AiService } from '@/services/ai.service'; +import type { UserService } from '@/services/user.service'; + +import { FreeAiCreditsService } from '../free-ai-credits.service'; + +describe('FreeAiCreditsService', () => { + const licenseState = mock(); + const globalConfig = mock({ + aiAssistant: { baseUrl: 'https://ai-assistant.n8n.io' }, + }); + const aiService = mock(); + const credentialsService = mock(); + const userService = mock(); + + const service = new FreeAiCreditsService( + licenseState, + globalConfig, + aiService, + credentialsService, + userService, + ); + + beforeEach(() => { + vi.clearAllMocks(); + licenseState.isAiCreditsLicensed.mockReturnValue(true); + licenseState.isAiGatewayLicensed.mockReturnValue(false); + globalConfig.aiAssistant.baseUrl = 'https://ai-assistant.n8n.io'; + }); + + describe('isEligible', () => { + it('returns true when AI credits are licensed, the base URL is set, the gateway is not licensed, and the user has not claimed', () => { + const user = { settings: {} } as User; + + expect(service.isEligible(user)).toBe(true); + }); + + it('returns false when AI credits are not licensed', () => { + licenseState.isAiCreditsLicensed.mockReturnValue(false); + const user = { settings: {} } as User; + + expect(service.isEligible(user)).toBe(false); + }); + + it('returns false when the AI assistant base URL is not configured', () => { + globalConfig.aiAssistant.baseUrl = ''; + const user = { settings: {} } as User; + + expect(service.isEligible(user)).toBe(false); + + globalConfig.aiAssistant.baseUrl = 'https://ai-assistant.n8n.io'; + }); + + it('returns false when the AI gateway is licensed', () => { + licenseState.isAiGatewayLicensed.mockReturnValue(true); + const user = { settings: {} } as User; + + expect(service.isEligible(user)).toBe(false); + }); + + it('returns false when the user already claimed free credits', () => { + const user = { settings: { userClaimedAiCredits: true } } as User; + + expect(service.isEligible(user)).toBe(false); + }); + }); + + describe('claim', () => { + it('generates credits, creates a managed credential, and marks the user as claimed', async () => { + const user = { id: 'user123', settings: {} } as User; + aiService.createFreeAiCredits.mockResolvedValue({ + apiKey: 'sk-test', + url: 'https://proxy.n8n.io', + }); + const credential = mock>>(); + credentialsService.createManagedCredential.mockResolvedValue(credential); + + const result = await service.claim(user, 'project123'); + + expect(aiService.createFreeAiCredits).toHaveBeenCalledWith(user); + expect(credentialsService.createManagedCredential).toHaveBeenCalledWith( + { + name: FREE_AI_CREDITS_CREDENTIAL_NAME, + type: 'openAiApi', + data: { apiKey: 'sk-test', url: 'https://proxy.n8n.io' }, + projectId: 'project123', + }, + user, + ); + expect(userService.updateSettings).toHaveBeenCalledWith('user123', { + userClaimedAiCredits: true, + }); + expect(result).toBe(credential); + }); + }); +}); diff --git a/packages/cli/src/services/free-ai-credits.service.ts b/packages/cli/src/services/free-ai-credits.service.ts new file mode 100644 index 00000000000..32f4b913e68 --- /dev/null +++ b/packages/cli/src/services/free-ai-credits.service.ts @@ -0,0 +1,58 @@ +import { LicenseState } from '@n8n/backend-common'; +import { GlobalConfig } from '@n8n/config'; +import type { User } from '@n8n/db'; +import { Service } from '@n8n/di'; +import { OPEN_AI_API_CREDENTIAL_TYPE } from 'n8n-workflow'; + +import { FREE_AI_CREDITS_CREDENTIAL_NAME } from '@/constants'; +import { CredentialsService } from '@/credentials/credentials.service'; +import { AiService } from '@/services/ai.service'; +import { UserService } from '@/services/user.service'; + +/** + * Provisions the free OpenAI credits managed credential. Backs both the + * `POST /ai/free-credits` route (browser-initiated claim) and the agent + * builder's `resolve_llm` tool (server-initiated silent claim). + */ +@Service() +export class FreeAiCreditsService { + constructor( + private readonly licenseState: LicenseState, + private readonly globalConfig: GlobalConfig, + private readonly aiService: AiService, + private readonly credentialsService: CredentialsService, + private readonly userService: UserService, + ) {} + + /** Whether this user can claim free OpenAI credits right now. */ + isEligible(user: User): boolean { + return ( + this.licenseState.isAiCreditsLicensed() && + !!this.globalConfig.aiAssistant.baseUrl && + !this.licenseState.isAiGatewayLicensed() && + !user.settings?.userClaimedAiCredits + ); + } + + /** Claims free credits: provisions the managed openAiApi credential and marks the user as claimed. */ + async claim(user: User, projectId?: string) { + const aiCredits = await this.aiService.createFreeAiCredits(user); + + const credential = await this.credentialsService.createManagedCredential( + { + name: FREE_AI_CREDITS_CREDENTIAL_NAME, + type: OPEN_AI_API_CREDENTIAL_TYPE, + data: { + apiKey: aiCredits.apiKey, + url: aiCredits.url, + }, + projectId, + }, + user, + ); + + await this.userService.updateSettings(user.id, { userClaimedAiCredits: true }); + + return credential; + } +} diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index e47ac0a8778..46d3ee13799 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -6467,6 +6467,7 @@ "instanceAi.credential.selected": "Credential selected", "instanceAi.credential.required": "This action requires a credential", "instanceAi.credential.allSelected": "All credentials configured", + "instanceAi.credential.someSkipped": "Credentials configured — skipped ones can be added later", "instanceAi.credential.useSelected": "Use selected", "instanceAi.credential.deny": "Later", "instanceAi.credential.confirmAll": "Confirm all credentials", diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts index 9cbd684e84b..72634835dfc 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts @@ -854,7 +854,6 @@ describe('AgentBuilderView — preview routing', () => { artifactMode: true, artifactProjectId: 'p2', artifactAgentId: 'a2', - artifactRefreshKey: 0, }, }); const header = wrapper.findComponent({ name: 'AgentBuilderHeader' }); @@ -1178,7 +1177,6 @@ describe('AgentBuilderView — three-column shell', () => { artifactMode: true, artifactProjectId: 'p2', artifactAgentId: 'a2', - artifactRefreshKey: 0, }, }); @@ -1218,7 +1216,6 @@ describe('AgentBuilderView — three-column shell', () => { artifactMode: true, artifactProjectId: 'p2', artifactAgentId: 'a2', - artifactRefreshKey: 0, }, }); @@ -1232,13 +1229,39 @@ describe('AgentBuilderView — three-column shell', () => { }); }); + it('drops a config edit queued right before the artifact lock engages instead of persisting it', async () => { + const wrapper = await renderView({ + props: { + artifactMode: true, + artifactProjectId: 'p2', + artifactAgentId: 'a2', + artifactEditingLocked: false, + }, + }); + updateConfigMock.mockClear(); + + vi.useFakeTimers(); + try { + wrapper + .findComponent({ name: 'AgentBuilderEditorColumn' }) + .vm.$emit('update:config', { name: 'Renamed while building' }); + + await wrapper.setProps({ artifactEditingLocked: true }); + await vi.advanceTimersByTimeAsync(500); + } finally { + vi.useRealTimers(); + } + await flushPromises(); + + expect(updateConfigMock).not.toHaveBeenCalled(); + }); + it('keeps artifact mode tab switching out of the route query', async () => { const wrapper = await renderView({ props: { artifactMode: true, artifactProjectId: 'p2', artifactAgentId: 'a2', - artifactRefreshKey: 0, }, }); routerReplace.mockClear(); @@ -1260,7 +1283,6 @@ describe('AgentBuilderView — three-column shell', () => { artifactMode: true, artifactProjectId: 'p2', artifactAgentId: 'a2', - artifactRefreshKey: 0, }, }); @@ -1278,25 +1300,6 @@ describe('AgentBuilderView — three-column shell', () => { }); }); - it('refreshes the artifact shell when the artifact refresh key changes', async () => { - const wrapper = await renderView({ - props: { - artifactMode: true, - artifactProjectId: 'p2', - artifactAgentId: 'a2', - artifactRefreshKey: 0, - }, - }); - getAgentMock.mockClear(); - fetchConfigMock.mockClear(); - - await wrapper.setProps({ artifactRefreshKey: 1 }); - await flushPromises(); - - expect(getAgentMock).toHaveBeenCalledWith({ baseUrl: 'http://localhost:5678' }, 'p2', 'a2'); - expect(fetchConfigMock).toHaveBeenCalledWith('p2', 'a2'); - }); - it('refreshes the shell when another surface reports an update to this agent', async () => { // Unique ids: earlier tests leave mounted instances (and their bus // listeners) behind, so shared ids would inflate the mock call counts. @@ -1305,13 +1308,18 @@ describe('AgentBuilderView — three-column shell', () => { artifactMode: true, artifactProjectId: 'p-bus', artifactAgentId: 'a-bus', - artifactRefreshKey: 0, }, }); getAgentMock.mockClear(); fetchConfigMock.mockClear(); - agentsEventBus.emit('agentUpdated', { agentId: 'a-bus', source: 'channel-setup-card' }); + vi.useFakeTimers(); + try { + agentsEventBus.emit('agentUpdated', { agentId: 'a-bus', source: 'channel-setup-card' }); + await vi.advanceTimersByTimeAsync(400); + } finally { + vi.useRealTimers(); + } await flushPromises(); expect(getAgentMock).toHaveBeenCalledWith( @@ -1324,8 +1332,14 @@ describe('AgentBuilderView — three-column shell', () => { // Other agents' updates and the builder's own writes are ignored. getAgentMock.mockClear(); fetchConfigMock.mockClear(); - agentsEventBus.emit('agentUpdated', { agentId: 'a-other', source: 'channel-setup-card' }); - agentsEventBus.emit('agentUpdated', { agentId: 'a-bus', source: 'agent-builder' }); + vi.useFakeTimers(); + try { + agentsEventBus.emit('agentUpdated', { agentId: 'a-other', source: 'channel-setup-card' }); + agentsEventBus.emit('agentUpdated', { agentId: 'a-bus', source: 'agent-builder' }); + await vi.advanceTimersByTimeAsync(400); + } finally { + vi.useRealTimers(); + } await flushPromises(); expect(getAgentMock).not.toHaveBeenCalled(); @@ -1334,6 +1348,33 @@ describe('AgentBuilderView — three-column shell', () => { wrapper.unmount(); }); + it('coalesces rapid external agent updates into one refresh cascade', async () => { + const wrapper = await renderView({ + props: { + artifactMode: true, + artifactProjectId: 'p-debounce', + artifactAgentId: 'a-debounce', + }, + }); + getAgentMock.mockClear(); + fetchConfigMock.mockClear(); + + vi.useFakeTimers(); + try { + agentsEventBus.emit('agentUpdated', { agentId: 'a-debounce', source: 'channel-setup-card' }); + agentsEventBus.emit('agentUpdated', { agentId: 'a-debounce', source: 'instance-ai' }); + await vi.advanceTimersByTimeAsync(400); + } finally { + vi.useRealTimers(); + } + await flushPromises(); + + expect(getAgentMock).toHaveBeenCalledTimes(1); + expect(fetchConfigMock).toHaveBeenCalledTimes(1); + + wrapper.unmount(); + }); + it('replays external agent updates that arrive before initialization completes', async () => { let resolveAgent!: (agent: ReturnType) => void; getAgentMock.mockReturnValueOnce(new Promise((resolve) => (resolveAgent = resolve))); @@ -1345,7 +1386,6 @@ describe('AgentBuilderView — three-column shell', () => { artifactMode: true, artifactProjectId: 'p-bus-init', artifactAgentId: 'a-bus-init', - artifactRefreshKey: 0, }, }); await vi.waitFor(() => { @@ -1374,45 +1414,7 @@ describe('AgentBuilderView — three-column shell', () => { wrapper.unmount(); }); - it('replays artifact refresh key changes that arrive before initialization completes', async () => { - let resolveAgent!: (agent: ReturnType) => void; - getAgentMock.mockReturnValueOnce(new Promise((resolve) => (resolveAgent = resolve))); - - const wrapper = await renderView({ - waitForAsyncSetup: false, - props: { - artifactMode: true, - artifactProjectId: 'p2', - artifactAgentId: 'a2', - artifactRefreshKey: 0, - }, - }); - await vi.waitFor(() => { - expect(getAgentMock).toHaveBeenCalledTimes(1); - expect(fetchConfigMock).toHaveBeenCalledTimes(1); - }); - - await wrapper.setProps({ artifactRefreshKey: 1 }); - await nextTick(); - expect(getAgentMock).toHaveBeenCalledTimes(1); - expect(fetchConfigMock).toHaveBeenCalledTimes(1); - - await wrapper.setProps({ artifactRefreshKey: 2 }); - await nextTick(); - expect(getAgentMock).toHaveBeenCalledTimes(1); - expect(fetchConfigMock).toHaveBeenCalledTimes(1); - - resolveAgent(makeAgentResponse()); - await flushPromises(); - await flushPromises(); - - expect(getAgentMock).toHaveBeenCalledTimes(2); - expect(fetchConfigMock).toHaveBeenCalledTimes(2); - expect(getAgentMock).toHaveBeenLastCalledWith({ baseUrl: 'http://localhost:5678' }, 'p2', 'a2'); - expect(fetchConfigMock).toHaveBeenLastCalledWith('p2', 'a2'); - }); - - it('surfaces errors from pending artifact refresh replay', async () => { + it('surfaces errors from a replayed external agent update', async () => { let resolveAgent!: (agent: ReturnType) => void; getAgentMock.mockReturnValueOnce(new Promise((resolve) => (resolveAgent = resolve))); fetchConfigMock.mockImplementationOnce(async () => { @@ -1425,24 +1427,23 @@ describe('AgentBuilderView — three-column shell', () => { waitForAsyncSetup: false, props: { artifactMode: true, - artifactProjectId: 'p2', - artifactAgentId: 'a2', - artifactRefreshKey: 0, + artifactProjectId: 'p-err', + artifactAgentId: 'a-err', }, }); await vi.waitFor(() => { - expect(getAgentMock).toHaveBeenCalledTimes(1); expect(fetchConfigMock).toHaveBeenCalledTimes(1); }); - await wrapper.setProps({ artifactRefreshKey: 1 }); - await nextTick(); + // Lands mid-initialize → queued via pendingExternalRefresh, replayed after init. + agentsEventBus.emit('agentUpdated', { agentId: 'a-err', source: 'channel-setup-card' }); resolveAgent(makeAgentResponse()); await flushPromises(); await flushPromises(); expect(showErrorMock).toHaveBeenCalledWith(replayError, 'agents.builder.loadError'); + wrapper.unmount(); }); it('adds JSON import and export actions to the header menu', async () => { diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentCapabilitiesSection.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentCapabilitiesSection.test.ts index d7fdf7c6240..4d95a82ee9e 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentCapabilitiesSection.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentCapabilitiesSection.test.ts @@ -638,15 +638,101 @@ describe('AgentCapabilitiesSection', () => { expect(wrapper.emitted('tasks-changed')).toEqual([[]]); }); - it('hides the add-tool and add-skill buttons when disabled (read-only host)', async () => { - const wrapper = mountSection([]); + it('disables the add-tool and add-skill buttons when disabled (read-only host)', async () => { + const wrapper = mountSection( + [], + {}, + configWithMcpServers([ + { + name: 'github', + url: 'https://mcp.github.com', + transport: 'streamableHttp', + authentication: 'none', + }, + ]), + [], + [], + { + skills: [ + { + id: 'skill-1', + skill: { name: 'Refund policy', description: '', instructions: '' }, + }, + ], + }, + ); + await flushPromises(); + expect(wrapper.find('[data-testid="agent-capabilities-add-tool"]').exists()).toBe(true); expect(wrapper.find('[data-testid="agent-capabilities-add-skill"]').exists()).toBe(true); + expect( + wrapper.find('[data-testid="agent-capabilities-add-tool"]').attributes('disabled'), + ).toBeUndefined(); + expect( + wrapper.find('[data-testid="agent-capabilities-add-skill"]').attributes('disabled'), + ).toBeUndefined(); await wrapper.setProps({ disabled: true }); - expect(wrapper.find('[data-testid="agent-capabilities-add-tool"]').exists()).toBe(false); - expect(wrapper.find('[data-testid="agent-capabilities-add-skill"]').exists()).toBe(false); + expect(wrapper.find('[data-testid="agent-capabilities-add-tool"]').exists()).toBe(true); + expect(wrapper.find('[data-testid="agent-capabilities-add-skill"]').exists()).toBe(true); + expect( + wrapper.find('[data-testid="agent-capabilities-add-tool"]').attributes('disabled'), + ).toBeDefined(); + expect( + wrapper.find('[data-testid="agent-capabilities-add-skill"]').attributes('disabled'), + ).toBeDefined(); + + const toolChip = wrapper.find('[data-testid="agent-capabilities-tool-row"]'); + const skillChip = wrapper.find('[data-testid="agent-capabilities-skill-row"]'); + expect(toolChip.attributes('disabled')).toBeDefined(); + expect(skillChip.attributes('disabled')).toBeDefined(); + + await toolChip.trigger('click'); + await skillChip.trigger('click'); + + expect(wrapper.emitted('open-tool')).toBeUndefined(); + expect(wrapper.emitted('open-skill')).toBeUndefined(); + }); + + it('disables the grouped-tool dropdown menu when disabled (read-only host)', async () => { + getNodeType.mockImplementation((type: string) => { + if (type === 'n8n-nodes-base.gmailTool') { + return createNodeType('n8n-nodes-base.gmailTool', 'Gmail Tool'); + } + + return null; + }); + + const wrapper = mountSection([ + { + type: 'node', + name: 'inbox_triage', + node: { + nodeType: 'n8n-nodes-base.gmailTool', + nodeTypeVersion: 1, + nodeParameters: {}, + }, + }, + { + type: 'node', + name: 'send_follow_up', + node: { + nodeType: 'n8n-nodes-base.gmailTool', + nodeTypeVersion: 1, + nodeParameters: {}, + }, + }, + ]); + + // Reka's DropdownMenuTrigger — not the read-only chip inside it — is what + // actually gates opening the menu, so assert its own disabled state. + const trigger = wrapper.find('[aria-haspopup="menu"]'); + expect(trigger.attributes('disabled')).toBe('false'); + + await wrapper.setProps({ disabled: true }); + + expect(wrapper.find('[aria-haspopup="menu"]').attributes('disabled')).toBe('true'); }); describe('channel modal', () => { diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChannelSetup.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChannelSetup.test.ts index 3bee2c04ddb..eba168f2e3b 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChannelSetup.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChannelSetup.test.ts @@ -19,6 +19,10 @@ const { setCredentialsMock: vi.fn(), })); +vi.mock('../composables/useAgentApi', () => ({ + createSlackAgentApp: vi.fn().mockResolvedValue({ installUrl: 'https://slack.test/install' }), +})); + vi.mock('@n8n/stores/useRootStore', () => ({ useRootStore: () => ({ restApiContext: {} }), })); @@ -97,4 +101,55 @@ describe('useAgentChannelSetup', () => { expect(fetchProjectMock).toHaveBeenCalledWith('artifact-project'); expect(credentialPermissions.value.create).toBe(true); }); + + it('resolves setupSlackApp successfully when the popup closes while an in-flight poll is about to confirm the connection', async () => { + vi.useFakeTimers(); + + class FakeBroadcastChannel { + addEventListener() {} + close() {} + postMessage() {} + } + vi.stubGlobal('BroadcastChannel', FakeBroadcastChannel); + + const fakePopup = { closed: false, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(fakePopup as unknown as Window); + + let resolveFirstPoll!: () => void; + const firstPoll = new Promise((resolve) => { + resolveFirstPoll = resolve; + }); + let isConnected = false; + const fetchStatus = vi.fn().mockImplementation(async () => { + if (fetchStatus.mock.calls.length === 1) { + await firstPoll; + } + }); + + const onConnected = vi.fn(); + const { setupSlackApp } = useAgentChannelSetup({ + projectId: () => 'artifact-project', + agentId: () => 'agent-1', + currentIntegration: null, + connectedCredentials: {}, + fetchStatus, + isIntegrationConnected: () => isConnected, + }); + + const setupPromise = setupSlackApp('token', onConnected); + await vi.advanceTimersByTimeAsync(0); + + fakePopup.closed = true; + await vi.advanceTimersByTimeAsync(2000); + + isConnected = true; + resolveFirstPoll(); + + await expect(setupPromise).resolves.toBe(true); + expect(onConnected).toHaveBeenCalled(); + + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); }); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentConfigAutosave.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentConfigAutosave.test.ts index 31f07210123..8b346725e4d 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentConfigAutosave.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentConfigAutosave.test.ts @@ -50,11 +50,11 @@ describe('useAgentConfigAutosave', () => { let rejectFirstSave: (error: Error) => void = () => {}; const save = vi.fn((snapshot: { value: string }) => { if (snapshot.value === 'old') { - return new Promise((_resolve, reject) => { + return new Promise<'skipped' | undefined>((_resolve, reject) => { rejectFirstSave = reject; }); } - return Promise.resolve(); + return Promise.resolve(undefined); }); const autosave = useAgentConfigAutosave<{ value: string }>({ save, @@ -92,4 +92,23 @@ describe('useAgentConfigAutosave', () => { expect(save).not.toHaveBeenCalled(); }); + + it('keeps saveStatus idle and skips onSaved when save resolves "skipped"', async () => { + vi.useFakeTimers(); + const save = vi.fn().mockResolvedValue('skipped' as const); + const onSaved = vi.fn(); + const autosave = useAgentConfigAutosave<{ value: string }>({ + save, + onSaved, + debounceMs: 500, + }); + + autosave.scheduleAutosave({ value: 'latest' }); + await vi.advanceTimersByTimeAsync(500); + await autosave.settleAutosave(); + + expect(save).toHaveBeenCalledTimes(1); + expect(onSaved).not.toHaveBeenCalled(); + expect(autosave.saveStatus.value).toBe('idle'); + }); }); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentBuilderHeader.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentBuilderHeader.vue index e767ac211f8..4233ac277cd 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentBuilderHeader.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentBuilderHeader.vue @@ -38,6 +38,8 @@ const props = defineProps<{ beforeRevertToPublished?: () => Promise | void; isVersionHistoryOpen?: boolean; artifactMode?: boolean; + /** True while the AI is actively building/mutating this agent in artifact mode — disables publish/revert/unpublish without hiding them. */ + editingLocked?: boolean; configValidationStatus?: 'valid' | 'invalid' | null; beforePublish?: () => Promise; }>(); @@ -223,7 +225,7 @@ const isVersionHistoryDisabled = computed(() => !props.agent?.hasPublishHistory) :agent="agent" :project-id="projectId" :agent-id="agentId" - :is-saving="saveStatus === 'saving'" + :is-saving="saveStatus === 'saving' || editingLocked" :before-revert-to-published="beforeRevertToPublished" :config-validation-status="configValidationStatus" :before-publish="beforePublish" diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentCapabilitiesSection.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentCapabilitiesSection.vue index 6c083b4e2a2..28faeaad3d7 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentCapabilitiesSection.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentCapabilitiesSection.vue @@ -570,11 +570,7 @@ function handleChannelDisconnected(channelType: string) { @@ -715,6 +716,7 @@ function handleChannelDisconnected(channelType: string) { icon="sparkles" :invalid="(skillIssueMessages.get(id) ?? []).length > 0" :invalid-reasons="skillIssueMessages.get(id) ?? []" + :disabled="props.disabled" :class="$style.capabilityChip" data-testid="agent-capabilities-skill-row" @click="emit('open-skill', id)" @@ -723,7 +725,6 @@ function handleChannelDisconnected(channelType: string) { @@ -755,6 +757,7 @@ function handleChannelDisconnected(channelType: string) { icon="bot" :invalid="subAgent.invalid" :invalid-reasons="subAgent.invalidReasons" + :disabled="props.disabled" :class="$style.capabilityChip" data-testid="agent-capabilities-sub-agent-row" @click="openExistingSubAgentModal(subAgent)" @@ -792,6 +795,7 @@ function handleChannelDisconnected(channelType: string) { icon="clipboard-list" :invalid="task.invalid" :invalid-reasons="task.invalidReasons" + :disabled="props.disabled" :class="$style.capabilityChip" data-testid="agent-capabilities-task-row" @click="openTaskModal(task)" @@ -882,11 +886,6 @@ function handleChannelDisconnected(channelType: string) { gap: var(--spacing--4xs); } -.disabled { - opacity: 0.5; - pointer-events: none; -} - .error { color: var(--color--danger); } diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentMemoryPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentMemoryPanel.vue index 8aa1b193f5a..1b39e77ec8a 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentMemoryPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentMemoryPanel.vue @@ -203,7 +203,7 @@ function onEpisodicMemoryToggle(enabled: boolean) {