From fe2d54786074532d5a50df5683122a63cb21134c Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Fri, 10 Jul 2026 20:57:33 +0100 Subject: [PATCH] refactor(core): drop ask_llm in favor of the shared question flow Model selection now flows the same way on both chat surfaces: ask_questions (batched question card) resolves the user's choice, then resolve_llm validates provider/model and resolves the credential. The dedicated ask_llm picker tool, its FE card, and the instance-AI session-exclusion/bounce plumbing built around it are removed as redundant. Co-Authored-By: Claude Fable 5 --- .../src/agent-builder-interactive.ts | 25 --- packages/@n8n/api-types/src/agents/index.ts | 5 - packages/@n8n/instance-ai/docs/tools.md | 15 +- .../__tests__/build-agent.tool.test.ts | 193 ++---------------- .../tools/orchestration/build-agent.tool.ts | 131 ++++-------- ...stance-ai-builder-delegate.adapter.test.ts | 27 +-- .../agents-builder.service.session.test.ts | 25 +-- .../agents-builder-model-recommendations.ts | 5 +- .../agents/builder/agents-builder-prompts.ts | 52 ++--- .../builder/agents-builder-tools.service.ts | 2 - .../agents/builder/agents-builder.service.ts | 15 -- .../agents/builder/builder-tool-names.ts | 2 +- .../__tests__/ask-llm.tool.test.ts | 47 ----- .../builder/interactive/ask-llm.tool.ts | 34 --- .../agents/builder/interactive/index.ts | 1 - .../interactive/llm-provider-defaults.ts | 4 +- .../builder/interactive/resolve-llm.tool.ts | 4 +- .../builder/prompts/config-mutation.prompt.ts | 2 +- .../builder/prompts/config-rules.prompt.ts | 4 +- .../builder/prompts/llm-selection.prompt.ts | 20 +- .../agents/builder/prompts/memory.prompt.ts | 2 +- .../instance-ai-builder-delegate.adapter.ts | 28 +-- .../frontend/@n8n/i18n/src/locales/en.json | 1 - .../agents/__tests__/AgentChatPanel.test.ts | 44 ++-- .../agents/__tests__/AskLlmCard.test.ts | 181 ---------------- .../__tests__/agentChatMessages.test.ts | 103 ++++------ .../__tests__/interactive-summary.test.ts | 13 -- .../agents/__tests__/toolDisplayName.test.ts | 2 +- .../__tests__/useAgentChatStream.test.ts | 29 ++- .../components/interactive/AskLlmCard.vue | 150 -------------- .../interactive/InteractiveCard.vue | 19 +- .../src/features/agents/utils/model-string.ts | 4 +- .../__tests__/messageMappers.test.ts | 12 +- .../shared/agentsChat/interactiveSummary.ts | 9 - .../ai/shared/agentsChat/messageMappers.ts | 23 +-- .../features/ai/shared/agentsChat/types.ts | 12 +- 36 files changed, 210 insertions(+), 1035 deletions(-) delete mode 100644 packages/cli/src/modules/agents/builder/interactive/__tests__/ask-llm.tool.test.ts delete mode 100644 packages/cli/src/modules/agents/builder/interactive/ask-llm.tool.ts delete mode 100644 packages/frontend/editor-ui/src/features/agents/__tests__/AskLlmCard.test.ts delete mode 100644 packages/frontend/editor-ui/src/features/agents/components/interactive/AskLlmCard.vue diff --git a/packages/@n8n/api-types/src/agent-builder-interactive.ts b/packages/@n8n/api-types/src/agent-builder-interactive.ts index 8b61d997099..1e28801c3a3 100644 --- a/packages/@n8n/api-types/src/agent-builder-interactive.ts +++ b/packages/@n8n/api-types/src/agent-builder-interactive.ts @@ -13,7 +13,6 @@ import { * by it. There is no separate `interactionType` field — the tool name IS the * interaction kind. */ -export const ASK_LLM_TOOL_NAME = 'ask_llm' as const; export const ASK_CREDENTIAL_TOOL_NAME = 'ask_credential' as const; export const ASK_EMBEDDING_CREDENTIAL_TOOL_NAME = 'ask_embedding_credential' as const; export { ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME }; @@ -26,7 +25,6 @@ export { ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME }; export const APPROVAL_TOOL_NAME = 'approval' as const; export const interactiveToolNameSchema = z.union([ - z.literal(ASK_LLM_TOOL_NAME), z.literal(ASK_CREDENTIAL_TOOL_NAME), z.literal(ASK_EMBEDDING_CREDENTIAL_TOOL_NAME), z.literal(ASK_QUESTIONS_TOOL_NAME), @@ -35,29 +33,6 @@ export const interactiveToolNameSchema = z.union([ export type InteractiveToolName = z.infer; -// --------------------------------------------------------------------------- -// ask_llm -// --------------------------------------------------------------------------- - -export const askLlmInputSchema = z.object({ - purpose: z - .string() - .optional() - .describe( - 'Short sentence describing why the model is needed, e.g. "Main LLM for the Slack triage agent"', - ), -}); - -export const askLlmResumeSchema = z.object({ - provider: z.string(), - model: z.string(), - credentialId: z.string(), - credentialName: z.string(), -}); - -export type AskLlmInput = z.infer; -export type AskLlmResume = z.infer; - // --------------------------------------------------------------------------- // ask_credential // --------------------------------------------------------------------------- diff --git a/packages/@n8n/api-types/src/agents/index.ts b/packages/@n8n/api-types/src/agents/index.ts index 28d5b04a410..20398b4b264 100644 --- a/packages/@n8n/api-types/src/agents/index.ts +++ b/packages/@n8n/api-types/src/agents/index.ts @@ -17,18 +17,13 @@ export { // ASK_QUESTIONS_TOOL_NAME / CONFIGURE_CHANNEL_TOOL_NAME come from // ./agent-interaction.schema (re-exported below via `export *`). export { - ASK_LLM_TOOL_NAME, ASK_CREDENTIAL_TOOL_NAME, ASK_EMBEDDING_CREDENTIAL_TOOL_NAME, APPROVAL_TOOL_NAME, interactiveToolNameSchema, - askLlmInputSchema, - askLlmResumeSchema, askCredentialInputSchema, cancellationResumeSchema, type InteractiveToolName, - type AskLlmInput, - type AskLlmResume, type AskCredentialInput, type CancellationResumeData, } from '../agent-builder-interactive'; diff --git a/packages/@n8n/instance-ai/docs/tools.md b/packages/@n8n/instance-ai/docs/tools.md index fa951c7f75b..7c50006ed5f 100644 --- a/packages/@n8n/instance-ai/docs/tools.md +++ b/packages/@n8n/instance-ai/docs/tools.md @@ -698,14 +698,17 @@ builder UI. **Returns**: `{ ok: true, builderReply, configUpdated }` on success, or `{ ok: false, error }`. -`configure_channel` (mandatory channel-setup UX for chat integrations) and -`ask_questions` (batched multi-question card) are standard builder tools — -available in the agents-module builder's own UI chat too, not injected extras -for sub-agent runs. Their suspend/resume payloads already match instance AI's +`configure_channel` (mandatory channel-setup UX for chat integrations), +`ask_questions` (batched multi-question card), and `resolve_llm` (non-interactive +model/credential resolution) are standard builder tools — available in the +agents-module builder's own UI chat too, not injected extras for sub-agent +runs. Model selection goes through `ask_questions` followed by `resolve_llm`, +so there is no dedicated model-picker tool to special-case. Interactive +suspend/resume payloads already match instance AI's confirmation-request/confirm-response wire contract (`@n8n/api-types` `agent-interaction.schema.ts`), so `build-agent` cascades -them through unchanged. A slim session prompt addendum steers the builder -away from the one tool this surface can't show a card for (`ask_llm`). +them through unchanged — including any suspension this surface has no +dedicated card for, which falls back to the generic approval card. ## Other Domain Tools 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 ca367da8469..29611832f27 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 @@ -1,9 +1,7 @@ import { ASK_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME, - cancellationResumeSchema, channelConfigSchema, channelSuspendPayloadSchema, credentialSuspendPayloadSchema, @@ -674,149 +672,41 @@ describe('build-agent tool — interactive suspend/resume cascade', () => { expect(publishedEvents.filter((event) => event.type === 'agent-completed')).toHaveLength(1); }); - it('bounces an ask_llm suspension back into the builder with a cancellation resume, without suspending', async () => { - const { context, delegate } = makeContext(); - vi.mocked(delegate.createAgent).mockResolvedValue({ agentId: 'agent-1', projectId: 'proj-1' }); - vi.mocked(delegate.streamBuild).mockResolvedValue( - fakeStream( - [ - // `agentRunId` on cascading suspension results is always '' for - // builder turns — the stream never carries a real run id (see - // resumable-stream-executor.ts). Resume routing must come from - // `findOpenSuspension`, not this chunk. - suspensionChunk({ - toolCallId: 'call-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: { purpose: 'Main LLM' }, - }), - ], - '', - ), - ); - vi.mocked(delegate.findOpenSuspension).mockResolvedValue({ - runId: 'builder-run-1', - toolCallId: 'tc-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: {}, - }); - vi.mocked(delegate.resumeBuild).mockResolvedValue(fakeStream([], 'Got it, using Anthropic.')); - const suspendFn = vi.fn(); - - const result = await runToolWithCtx( - context, - { message: 'Build it', name: 'New Agent' }, - { resumeData: undefined, suspend: suspendFn }, - ); - - expect(delegate.resumeBuild).toHaveBeenCalledWith( - 'agent-1', - { - runId: 'builder-run-1', - toolCallId: 'tc-1', - resumeData: { - _type: 'agent.cancellation', - // Steers the builder to the ask_questions HITL card instead of a - // plain-text question that would end the turn. - message: expect.stringContaining('via the ask_questions tool') as string, - }, - }, - { threadId: 'ia-builder:thread-1:agent-1', modelConfig: context.modelId }, - ); - const [, resumeCall] = vi.mocked(delegate.resumeBuild).mock.calls[0]; - expect(cancellationResumeSchema.parse(resumeCall.resumeData)).toEqual(resumeCall.resumeData); - expect(suspendFn).not.toHaveBeenCalled(); - expect(result).toEqual({ - ok: true, - builderReply: 'Got it, using Anthropic.', - configUpdated: false, - }); - }); - - it('returns a friendly error and stops the loop when the ask_llm bounce cannot recover an open suspension', async () => { + it('cascades a suspension from an unrecognized tool through unchanged, with no special-casing by tool name', async () => { const { context, delegate, publishedEvents } = makeContext(); vi.mocked(delegate.createAgent).mockResolvedValue({ agentId: 'agent-1', projectId: 'proj-1' }); + const payload = { + message: 'The agent builder needs something this chat has no dedicated card for', + severity: 'info' as const, + somethingToolSpecific: { arbitrary: true }, + }; vi.mocked(delegate.streamBuild).mockResolvedValue( fakeStream( [ suspensionChunk({ toolCallId: 'call-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: { purpose: 'Main LLM' }, + toolName: 'some_future_interactive_tool', + suspendPayload: payload, }), ], '', ), ); - vi.mocked(delegate.findOpenSuspension).mockResolvedValue(null); const suspendFn = vi.fn(); - const result = await runToolWithCtx( + await runToolWithCtx( context, { message: 'Build it', name: 'New Agent' }, { resumeData: undefined, suspend: suspendFn }, ); - expect(result.ok).toBe(false); - expect(result.error).toMatch(/builder's question could not be recovered/); + expect(suspendFn).toHaveBeenCalledTimes(1); + expect(suspendFn.mock.calls[0][0]).toEqual({ + ...payload, + requestId: expect.any(String) as unknown, + }); expect(delegate.resumeBuild).not.toHaveBeenCalled(); - expect(suspendFn).not.toHaveBeenCalled(); - expect(publishedEvents.filter((event) => event.type === 'agent-spawned')).toHaveLength(1); - const completedEvents = publishedEvents.filter((event) => event.type === 'agent-completed'); - expect(completedEvents).toHaveLength(1); - expect('payload' in completedEvents[0] ? completedEvents[0].payload : undefined).toMatchObject({ - role: 'agent-builder', - error: result.error, - }); - }); - - it('stops bouncing ask_llm after 3 cancellations and returns a friendly error, publishing one agent-spawned and one agent-completed', async () => { - const { context, delegate, publishedEvents } = makeContext(); - vi.mocked(delegate.createAgent).mockResolvedValue({ agentId: 'agent-1', projectId: 'proj-1' }); - const askLlmSuspension = () => - fakeStream( - [ - suspensionChunk({ - toolCallId: 'call-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: { purpose: 'Main LLM' }, - }), - ], - '', - ); - vi.mocked(delegate.streamBuild).mockResolvedValue(askLlmSuspension()); - vi.mocked(delegate.findOpenSuspension).mockResolvedValue({ - runId: 'builder-run-1', - toolCallId: 'tc-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: {}, - }); - // A delegate that never accepts the "ask in plain text" cancellation and - // keeps re-suspending ask_llm on every resume. - vi.mocked(delegate.resumeBuild).mockImplementation(async () => { - await Promise.resolve(); - return askLlmSuspension(); - }); - - const result = await runTool(context, { message: 'Build it', name: 'New Agent' }); - - expect(delegate.resumeBuild).toHaveBeenCalledTimes(3); - expect(result).toEqual({ - ok: false, - error: - 'The agent builder repeatedly requested a model picker this chat cannot show. ' + - 'Tell the user to configure the builder model in the agents module settings.', - }); - expect(publishedEvents.filter((event) => event.type === 'agent-spawned')).toHaveLength(1); - const completedEvents = publishedEvents.filter((event) => event.type === 'agent-completed'); - expect(completedEvents).toHaveLength(1); - expect(completedEvents[0]).toMatchObject({ - type: 'agent-completed', - agentId: 'agent-builder:agent-1', - }); - expect('payload' in completedEvents[0] ? completedEvents[0].payload : undefined).toMatchObject({ - role: 'agent-builder', - error: result.error, - }); + expect(publishedEvents.map((event) => event.type)).toEqual(['agent-spawned']); }); it('returns a friendly error when resuming and the builder question is no longer open', async () => { @@ -853,59 +743,6 @@ describe('build-agent tool — interactive suspend/resume cascade', () => { expect(delegate.findOpenSuspension).not.toHaveBeenCalled(); }); - it('suspends on a second interactive question after bouncing an ask_llm suspension internally', async () => { - const { context, delegate } = makeContext(); - vi.mocked(delegate.createAgent).mockResolvedValue({ agentId: 'agent-1', projectId: 'proj-1' }); - vi.mocked(delegate.streamBuild).mockResolvedValue( - fakeStream( - [ - suspensionChunk({ - toolCallId: 'call-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: {}, - }), - ], - '', - ), - ); - vi.mocked(delegate.findOpenSuspension).mockResolvedValue({ - runId: 'builder-run-1', - toolCallId: 'tc-1', - toolName: ASK_LLM_TOOL_NAME, - suspendPayload: {}, - }); - const secondPayload = askQuestionsSuspendPayload({ - message: 'Continue?', - questions: [{ id: 'q1', question: 'Continue?', type: 'text' }], - }); - vi.mocked(delegate.resumeBuild).mockResolvedValue( - fakeStream( - [ - suspensionChunk({ - toolCallId: 'call-2', - toolName: ASK_QUESTIONS_TOOL_NAME, - suspendPayload: secondPayload, - }), - ], - '', - ), - ); - const suspendFn = vi.fn(); - - await runToolWithCtx( - context, - { message: 'Build it', name: 'New Agent' }, - { resumeData: undefined, suspend: suspendFn }, - ); - - expect(delegate.resumeBuild).toHaveBeenCalledTimes(1); - expect(suspendFn).toHaveBeenCalledTimes(1); - expect(suspendFn.mock.calls[0][0]).toEqual({ - ...secondPayload, - requestId: expect.any(String) as unknown, - }); - }); - it('publishes agent-completed and rethrows when consumeStreamCascading itself throws mid-loop', async () => { const { context, delegate, publishedEvents } = makeContext(); vi.mocked(delegate.createAgent).mockResolvedValue({ agentId: 'agent-1', projectId: 'proj-1' }); 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 51eea8a1f14..7ef38b9438c 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 @@ -12,7 +12,6 @@ * builder UI — it is a private sub-agent conversation. */ import { Tool, type InterruptibleToolContext } from '@n8n/agents'; -import { ASK_LLM_TOOL_NAME, type CancellationResumeData } from '@n8n/api-types'; import { isRecord } from '@n8n/utils/is-record'; import { nanoid } from 'nanoid'; import { z } from 'zod'; @@ -79,11 +78,6 @@ function buildOutboundMessage(message: string, workflowContext?: SessionWorkflow return `${message}\n\n${formatWorkflowContextEnvelope(workflowContext)}`; } -/** Cancellation resume for interactions the builder can't surface yet (e.g. `ask_llm`, until its FE kind ships). */ -function builderCancellationResume(message: string): CancellationResumeData { - return { _type: 'agent.cancellation', message }; -} - const buildAgentInputSchema = z.object({ message: z .string() @@ -205,14 +199,6 @@ async function finishTurn( return { ok: false, error }; } -/** - * Cap on ask_llm cancellation bounces (see below) per invocation of the - * consume loop. Guards against a builder that keeps re-suspending ask_llm - * (e.g. it never accepts the "ask via ask_questions" cancellation) bouncing - * forever instead of terminating the turn. - */ -const ASK_LLM_BOUNCE_LIMIT = 3; - /** * Drain one builder turn, cascading any interactive suspension into this * tool's own `ctx.suspend()`. Shared by the first-call and resume legs so @@ -227,88 +213,45 @@ async function runBuilderConsumeLoop(params: { builderAgentId: string; initialTurn: BuilderTurnStream; }): Promise { - const { context, delegate, ctx, target, session, builderAgentId } = params; - let turn = params.initialTurn; - let askLlmBounceCount = 0; + const { context, ctx, builderAgentId, initialTurn: turn } = params; - for (;;) { - let result: ConsumeStreamCascadingResult; - try { - result = await consumeStreamCascading({ - agent: undefined, - stream: turn, - runId: context.runId, - agentId: builderAgentId, - eventBus: context.eventBus, - logger: context.logger, - threadId: context.threadId, - abortSignal: context.abortSignal, - }); - } catch (error) { - publishAgentBuilderFailure(context, builderAgentId, error); - throw error; - } - - if (result.status !== 'suspended') return await finishTurn(context, builderAgentId, result); - - const toolName = result.suspension.toolName ?? ''; - if (toolName === ASK_LLM_TOOL_NAME) { - askLlmBounceCount += 1; - if (askLlmBounceCount > ASK_LLM_BOUNCE_LIMIT) { - const message = - 'The agent builder repeatedly requested a model picker this chat cannot show. ' + - 'Tell the user to configure the builder model in the agents module settings.'; - publishAgentBuilderFailure(context, builderAgentId, new Error(message)); - return { ok: false, error: message }; - } - - // The stream never carries a real runId for builder turns (see - // resumable-stream-executor.ts), so resume routing must go through the - // same server-side checkpoint lookup the resume leg uses, not - // `result.agentRunId` (always `''`). - const open = await delegate.findOpenSuspension(target.agentId, session); - if (!open) { - const message = "The builder's question could not be recovered; the build turn was lost."; - publishAgentBuilderFailure(context, builderAgentId, new Error(message)); - return { ok: false, error: message }; - } - - // Temporary until the llm-picker FE kind ships: bounce back into the - // builder so it asks conversationally instead of dead-ending. - turn = await delegate.resumeBuild( - target.agentId, - { - runId: open.runId, - toolCallId: open.toolCallId, - resumeData: builderCancellationResume( - 'This chat cannot show the model picker. Ask which provider and model to use ' + - 'via the ask_questions tool (single-select options with a recommended ' + - 'default), then call resolve_llm with the choice to resolve the credential.', - ), - }, - session, - ); - continue; - } - - // The builder's interactive tools now natively emit the shared - // instance-AI confirmation payload shape, so this cascades the suspend - // payload straight through — only the requestId is re-minted at this - // (orchestrator) level. Still validated defensively: a malformed payload - // fails the turn instead of throwing out of the tool handler. - const suspendPayload = result.suspension.suspendPayload; - const parsedSuspend = isRecord(suspendPayload) - ? buildAgentSuspendSchema.safeParse({ ...suspendPayload, requestId: nanoid() }) - : undefined; - if (!parsedSuspend?.success) { - const message = - "The agent builder's confirmation request was malformed; the build turn was lost."; - publishAgentBuilderFailure(context, builderAgentId, new Error(message)); - return { ok: false, error: message }; - } - - return await ctx.suspend(parsedSuspend.data); + let result: ConsumeStreamCascadingResult; + try { + result = await consumeStreamCascading({ + agent: undefined, + stream: turn, + runId: context.runId, + agentId: builderAgentId, + eventBus: context.eventBus, + logger: context.logger, + threadId: context.threadId, + abortSignal: context.abortSignal, + }); + } catch (error) { + publishAgentBuilderFailure(context, builderAgentId, error); + throw error; } + + if (result.status !== 'suspended') return await finishTurn(context, builderAgentId, result); + + // Any interactive suspension — including one the FE has no dedicated card + // for — cascades through the generic passthrough: the suspend payload is + // forwarded as-is (only the requestId is re-minted at this orchestrator + // level) and the FE's approval-card fallback renders it. Still validated + // defensively: a malformed payload fails the turn instead of throwing out + // of the tool handler. + const suspendPayload = result.suspension.suspendPayload; + const parsedSuspend = isRecord(suspendPayload) + ? buildAgentSuspendSchema.safeParse({ ...suspendPayload, requestId: nanoid() }) + : undefined; + if (!parsedSuspend?.success) { + const message = + "The agent builder's confirmation request was malformed; the build turn was lost."; + publishAgentBuilderFailure(context, builderAgentId, new Error(message)); + return { ok: false, error: message }; + } + + return await ctx.suspend(parsedSuspend.data); } export function createBuildAgentTool(context: OrchestrationContext) { diff --git a/packages/cli/src/modules/agents/__tests__/instance-ai-builder-delegate.adapter.test.ts b/packages/cli/src/modules/agents/__tests__/instance-ai-builder-delegate.adapter.test.ts index da1e58069e7..51138f872ae 100644 --- a/packages/cli/src/modules/agents/__tests__/instance-ai-builder-delegate.adapter.test.ts +++ b/packages/cli/src/modules/agents/__tests__/instance-ai-builder-delegate.adapter.test.ts @@ -9,7 +9,6 @@ import type { AgentsService } from '../agents.service'; import type { AgentsBuilderService } from '../builder/agents-builder.service'; import type { Agent } from '../entities/agent.entity'; import { - BUILDER_EXCLUDED_TOOL_NAMES, INSTANCE_AI_BUILDER_ADDENDUM, InstanceAiBuilderDelegateAdapterService, } from '../instance-ai-builder-delegate.adapter'; @@ -33,23 +32,17 @@ function setup() { }; } -/** Extract the extraTools/addendum/excludeTools from a builder-service session-arg call. */ +/** Extract the addendum from a builder-service session-arg call. */ function sessionArg(call: unknown[]): { - extraTools?: Array<{ name: string }>; instructionsAddendum?: string; - excludeTools?: string[]; } { const session = call.at(-1) as | { - extraTools?: Array<{ name: string }>; instructionsAddendum?: string; - excludeTools?: string[]; } | undefined; return { - extraTools: session?.extraTools, instructionsAddendum: session?.instructionsAddendum, - excludeTools: session?.excludeTools, }; } @@ -118,20 +111,15 @@ describe('InstanceAiBuilderDelegateAdapterService', () => { expect(session?.modelConfig).toBe('anthropic/claude-sonnet-host-resolved'); }); - it('does not inject any extraTools — ask_questions/configure_channel are now standard builder tools', async () => { + it('passes the instance-AI session addendum through to the builder service', async () => { const { delegate, agentsBuilderService } = setup(); vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(true); agentsBuilderService.buildAgent.mockReturnValue(asAsyncGenerator([])); await delegate.streamBuild('agent-1', 'hi', { threadId: 'ia-builder:t:agent-1' }); - const { extraTools, instructionsAddendum, excludeTools } = sessionArg( - agentsBuilderService.buildAgent.mock.calls[0], - ); - expect(extraTools).toBeUndefined(); + const { instructionsAddendum } = sessionArg(agentsBuilderService.buildAgent.mock.calls[0]); expect(instructionsAddendum).toBe(INSTANCE_AI_BUILDER_ADDENDUM); - expect(excludeTools).toEqual(BUILDER_EXCLUDED_TOOL_NAMES); - expect(excludeTools).toContain('ask_llm'); }); it('rejects when the user lacks agent:update scope', async () => { @@ -186,7 +174,7 @@ describe('InstanceAiBuilderDelegateAdapterService', () => { expect(session?.modelConfig).toBe('anthropic/claude-sonnet-host-resolved'); }); - it('does not inject any extraTools — ask_questions/configure_channel are now standard builder tools', async () => { + it('passes the instance-AI session addendum through to the builder service', async () => { const { delegate, agentsBuilderService } = setup(); vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(true); agentsBuilderService.resumeBuild.mockReturnValue(asAsyncGenerator([])); @@ -197,13 +185,8 @@ describe('InstanceAiBuilderDelegateAdapterService', () => { { threadId: 'ia-builder:t:agent-1' }, ); - const { extraTools, instructionsAddendum, excludeTools } = sessionArg( - agentsBuilderService.resumeBuild.mock.calls[0], - ); - expect(extraTools).toBeUndefined(); + const { instructionsAddendum } = sessionArg(agentsBuilderService.resumeBuild.mock.calls[0]); expect(instructionsAddendum).toBe(INSTANCE_AI_BUILDER_ADDENDUM); - expect(excludeTools).toEqual(BUILDER_EXCLUDED_TOOL_NAMES); - expect(excludeTools).toContain('ask_llm'); }); it('rejects when the user lacks agent:update scope', async () => { 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 c9d9982e33e..bbc27a3d664 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 @@ -258,35 +258,16 @@ describe('AgentsBuilderService session isolation', () => { expect(agentsSdkMocks.instructionsCalls[0]).not.toContain('Extra sub-agent rules'); }); - it('omits standard tools named in session.excludeTools while still registering the rest', async () => { + it('registers all standard tools returned by the tools service', async () => { const { service, user, credentialProvider } = setup({ - json: [fakeTool('ask_llm'), fakeTool('read_config')], + json: [fakeTool('resolve_llm'), fakeTool('read_config')], shared: [fakeTool('ask_credential')], }); - await drain( - service.buildAgent('agent-1', 'project-1', 'hi', credentialProvider, user, { - threadId: 'ia-builder:t:agent-1', - excludeTools: ['ask_llm'], - }), - ); - - expect(agentsSdkMocks.registeredToolNames).not.toContain('ask_llm'); - expect(agentsSdkMocks.registeredToolNames).toEqual( - expect.arrayContaining(['read_config', 'ask_credential']), - ); - }); - - it('registers all standard tools when session.excludeTools is absent', async () => { - const { service, user, credentialProvider } = setup({ - json: [fakeTool('ask_llm'), fakeTool('read_config')], - shared: [], - }); - await drain(service.buildAgent('agent-1', 'project-1', 'hi', credentialProvider, user)); expect(agentsSdkMocks.registeredToolNames).toEqual( - expect.arrayContaining(['ask_llm', 'read_config']), + expect.arrayContaining(['resolve_llm', 'read_config', 'ask_credential']), ); }); 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 e65f1640d55..e37e7b86a72 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,10 +82,11 @@ 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_llm if the user needs to choose a credential. +provider and model, or ask via ask_questions if the user needs to choose a +credential. 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 or ask_llm result. +without a resolve_llm result. ${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 7cebd021920..22fbc706e8e 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts @@ -101,25 +101,27 @@ must be persisted exactly as returned. Once you are building, ask for any specific decision, choice, value, or clarification through one of these tools rather than in plain prose. Use -\`ask_llm\` for the model/credential, \`ask_credential\` for node-tool credentials, -\`configure_channel\` for chat-channel connections, and \`ask_questions\` for -everything else. 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". +\`ask_credential\` for node-tool credentials, \`configure_channel\` for +chat-channel connections, and \`ask_questions\` for everything else, including +the model/credential choice — resolve the answer with \`resolve_llm\`. +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". -- \`ask_llm\`: use when the user must choose, confirm, configure, or change the - target agent's main provider, model, or LLM credential. - \`ask_credential\`: use once per required node-tool credential slot before the config mutation that introduces the tool. 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. -- \`ask_questions\`: the default way to ask the user anything that isn't a model, - node-tool credential, or channel choice. Batch every 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. +- \`ask_questions\`: the default way to ask the user anything that isn't a + node-tool credential or channel choice, including when the user must choose, + confirm, configure, or change the target agent's main provider, model, or + LLM credential — resolve the answer with \`resolve_llm\`. Batch every + 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. - Never call two interactive tools in parallel. The run suspends on the first. - 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 @@ -173,17 +175,18 @@ change, call \`read_config\` again.`; export const IMPORTANT_SECTION = `\ ## Important -- Credentials are user-controlled. Use \`resolve_llm\` or \`ask_llm\` for the - target agent's main model, \`ask_credential\` for node-tool or Episodic Memory - credentials, and \`configure_channel\` (never \`ask_credential\`) for chat-channel +- Credentials are user-controlled. Use \`resolve_llm\` (asking via + \`ask_questions\` first when the user must choose) for the target agent's + main model, \`ask_credential\` for node-tool or Episodic Memory credentials, + and \`configure_channel\` (never \`ask_credential\`) for chat-channel credentials. Never copy credential IDs from \`list_credentials\` into config. - To get a specific decision, choice, or value for a build step, use \`ask_questions\` (discrete options for a known set, \`type: "text"\` for - open-ended; batch multiple questions into one call), or - \`ask_llm\`/\`ask_credential\`/\`configure_channel\` for model, credential, and - channel choices — not plain prose. Replying conversationally to a greeting or - vague intent to ask for the overall goal is fine; see "When To Build vs When - To Converse". + open-ended; batch multiple questions into one call) for model, credential, + and other choices, or \`ask_credential\`/\`configure_channel\` for node-tool + and channel credentials — not plain prose. Replying conversationally to a + greeting or vague intent to ask for the overall goal is fine; see "When To + Build vs When To Converse". - Tool preference order for real-world integrations: 1. MCP servers (\`search_mcp_servers\`) — always check first 2. Node tools (\`search_nodes\`) @@ -224,7 +227,8 @@ export const WORKFLOW_SECTION = `\ ## Workflow 1. If the agent has no \`instructions\` and \`credential\` yet, call - \`resolve_llm\` when the user specified a provider/model, or call \`ask_llm\` if user didn't specify a provider/model. + \`resolve_llm\` when the user specified a provider/model, or ask via + \`ask_questions\` and call \`resolve_llm\` with the answer if they didn't. 2. Draft real target-agent \`instructions\`; never write empty placeholders. 3. Use \`ask_questions\` for clarifying questions with discrete options, batching multiple questions into one call. @@ -242,7 +246,8 @@ export const FEW_SHOT_FLOWS_SECTION = `\ ## Example flows ### New agent: "Build me a Slack triage agent" -1. \`ask_llm({ purpose: "Choose a model" })\` -> resolved provider, model, and credential. +1. \`ask_questions({ ... })\` for the model choice, then + \`resolve_llm({ provider, model })\` -> resolved provider, model, and credential. 2. \`search_nodes({ query: "slack" })\`, then \`get_node_types(...)\`. 3. \`ask_credential(...)\` for the Slack credential slot. 4. \`read_config()\`. @@ -255,7 +260,8 @@ export const FEW_SHOT_FLOWS_SECTION = `\ \`credential\`, and requested instructions. ### Change the existing model -1. \`ask_llm({ purpose: "Choose a different model" })\`. +1. \`ask_questions({ ... })\` for the new model choice, then + \`resolve_llm({ provider, model })\`. 2. \`read_config()\`. 3. \`patch_config(...)\` replacing \`/model\` and \`/credential\`. diff --git a/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts b/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts index 332fed33c25..bb735be37a5 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 @@ -52,7 +52,6 @@ import { buildGetResourceLocatorOptionsTool } from './get-resource-locator-optio import { buildAskCredentialTool, buildAskEmbeddingCredentialTool, - buildAskLlmTool, buildAskQuestionsTool, buildConfigureChannelTool, buildResolveLlmTool, @@ -514,7 +513,6 @@ export class AgentsBuilderToolsService { isCredentialTypeKnown: (credentialType) => this.credentialTypes.recognizes(credentialType), isAssistantProxyEnabled: () => this.aiService.isProxyEnabled(), }), - buildAskLlmTool(), buildAskQuestionsTool(), buildConfigureChannelTool({ agentId, 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 de27fb5f175..07777509d17 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder.service.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder.service.ts @@ -1,5 +1,4 @@ import type { - BuiltTool, CredentialProvider, ModelConfig, SerializableAgentState, @@ -41,16 +40,8 @@ interface FindSuspendedCheckpointOptions { export interface BuilderSessionOptions { /** Overrides the persistence thread id. Default: `builder:`. */ threadId?: string; - /** Extra tools registered on the builder agent (e.g. instance-AI-injected). */ - extraTools?: BuiltTool[]; /** Extra text appended to the builder prompt (e.g. instance-AI sub-agent rules). */ instructionsAddendum?: string; - /** - * Tool names to omit for this session, e.g. interactive tools with no UI on - * the host surface. Only filters the standard tool set — `extraTools` are - * always registered regardless of this list. - */ - excludeTools?: string[]; /** * Overrides model resolution for this session — when set, the builder runs * on this model directly instead of `AgentsBuilderSettingsService.resolveModelConfig`. @@ -276,13 +267,7 @@ export class AgentsBuilderService { }); if (telemetry) builder.telemetry(telemetry); - const excludeTools = new Set(session?.excludeTools ?? []); for (const tool of [...tools.json, ...tools.shared]) { - if (excludeTools.has(tool.name)) continue; - builder.tool(tool); - } - - for (const tool of session?.extraTools ?? []) { builder.tool(tool); } 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 797387e103d..4ed34560d08 100644 --- a/packages/cli/src/modules/agents/builder/builder-tool-names.ts +++ b/packages/cli/src/modules/agents/builder/builder-tool-names.ts @@ -2,7 +2,7 @@ * Tool names used by the agent builder. Centralised so prompts, the SSE event * routing, and tests can't drift on string typos. * - * The interactive tools (`ask_llm`, `ask_credential`, `ask_embedding_credential`, + * The interactive tools (`ask_credential`, `ask_embedding_credential`, * `ask_questions`, `configure_channel`) are NOT listed here — their names live * in `@n8n/api-types` (`agent-builder-interactive.ts` / `agents/agent-interaction.schema.ts`) * alongside the suspend/resume schemas they share with instance AI's FE cards. diff --git a/packages/cli/src/modules/agents/builder/interactive/__tests__/ask-llm.tool.test.ts b/packages/cli/src/modules/agents/builder/interactive/__tests__/ask-llm.tool.test.ts deleted file mode 100644 index a52ceae103e..00000000000 --- a/packages/cli/src/modules/agents/builder/interactive/__tests__/ask-llm.tool.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import type { Mock } from 'vitest'; - -import { buildAskLlmTool } from '../ask-llm.tool'; - -interface TestCtx { - resumeData?: unknown; - suspend: Mock; -} - -function makeCtx(overrides?: { resumeData?: unknown }): TestCtx { - return { resumeData: overrides?.resumeData, suspend: vi.fn(async (x: unknown) => x) }; -} - -describe('ask_llm tool', () => { - it('instructs the builder to render the picker instead of asking in prose', () => { - const tool = buildAskLlmTool(); - expect(tool.systemInstruction).toContain('Never ask the user in plain text'); - expect(tool.systemInstruction).toContain('call ask_llm'); - }); - - it('suspends on first invocation so the user can choose', async () => { - const tool = buildAskLlmTool(); - const ctx = makeCtx(); - await tool.handler!({ purpose: 'Main LLM' }, ctx as never); - expect(ctx.suspend).toHaveBeenCalledWith({ purpose: 'Main LLM' }); - }); - - it('returns resumeData verbatim after resume', async () => { - const tool = buildAskLlmTool(); - const ctx = makeCtx({ - resumeData: { - provider: 'anthropic', - model: 'claude-sonnet-4-6', - credentialId: 'cX', - credentialName: 'Picked', - }, - }); - const result = await tool.handler!({ purpose: 'Main LLM' }, ctx as never); - expect(ctx.suspend).not.toHaveBeenCalled(); - expect(result).toEqual({ - provider: 'anthropic', - model: 'claude-sonnet-4-6', - credentialId: 'cX', - credentialName: 'Picked', - }); - }); -}); diff --git a/packages/cli/src/modules/agents/builder/interactive/ask-llm.tool.ts b/packages/cli/src/modules/agents/builder/interactive/ask-llm.tool.ts deleted file mode 100644 index 308a9bb2e0c..00000000000 --- a/packages/cli/src/modules/agents/builder/interactive/ask-llm.tool.ts +++ /dev/null @@ -1,34 +0,0 @@ -import type { BuiltTool, InterruptibleToolContext } from '@n8n/agents'; -import { Tool } from '@n8n/agents/tool'; -import { - ASK_LLM_TOOL_NAME, - askLlmInputSchema, - askLlmResumeSchema, - type AskLlmInput, - type AskLlmResume, -} from '@n8n/api-types'; - -export function buildAskLlmTool(): BuiltTool { - return new Tool(ASK_LLM_TOOL_NAME) - .description( - 'Show a model + credential picker card in the chat UI and suspend until the user ' + - 'selects a provider, model and credential. ' + - 'After resume: set model = "{provider}/{model}" and credential = credentialId ' + - 'via write_config or patch_config.', - ) - .systemInstruction( - 'Never ask the user in plain text to choose, confirm, configure, or change the agent ' + - 'main LLM, provider, model, or main LLM credential. If the user needs to make that ' + - 'choice, call ask_llm so the picker card is shown.', - ) - .input(askLlmInputSchema) - .suspend(askLlmInputSchema) - .resume(askLlmResumeSchema) - .handler( - async (input: AskLlmInput, ctx: InterruptibleToolContext) => { - if (ctx.resumeData !== undefined) return ctx.resumeData; - return await ctx.suspend(input); - }, - ) - .build(); -} diff --git a/packages/cli/src/modules/agents/builder/interactive/index.ts b/packages/cli/src/modules/agents/builder/interactive/index.ts index 9044e65269c..2c4c6ef1068 100644 --- a/packages/cli/src/modules/agents/builder/interactive/index.ts +++ b/packages/cli/src/modules/agents/builder/interactive/index.ts @@ -1,5 +1,4 @@ export { buildAskCredentialTool, buildAskEmbeddingCredentialTool } from './ask-credential.tool'; -export { buildAskLlmTool } from './ask-llm.tool'; export { buildAskQuestionsTool } from './ask-questions.tool'; export { buildConfigureChannelTool } from './configure-channel.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 acfc45e5365..3e807dce9fe 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 @@ -1,7 +1,7 @@ /** * Canonical "if you have one of THIS credential type, this is the LLM provider - * + model the builder may select when auto-resolving." Used by the ask_llm tool - * when there's exactly one LLM-provider credential available. + * + model the builder may select when auto-resolving." Used by the resolve_llm + * tool when there's exactly one LLM-provider credential available. * * Provider strings match the provider IDs used by `@n8n/agents`'s * `.model(provider, model)` call. 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 e9d2cf464ab..915fd076377 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 @@ -97,8 +97,8 @@ export function buildResolveLlmTool(deps: ResolveLlmToolDeps): BuiltTool { '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_llm only when the ' + - 'user must choose.', + 'when credentials are missing, unsupported, or ambiguous; use ask_questions to let the ' + + 'user choose, then call resolve_llm again with the choice.', ) .input( z.object({ 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 ad809ce073f..221b7429d7c 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 @@ -30,7 +30,7 @@ ${getSchemaReferenceSection()} - Keep each feature in the schema path where it belongs. - Preserve unrelated existing config unless the user asked to change it. - Never write placeholder instructions, tool descriptions, or skill descriptions. -- Never copy credential IDs from \`list_credentials\`; use \`resolve_llm\`, \`ask_llm\`, or \`ask_credential\`. +- Never copy credential IDs from \`list_credentials\`; use \`resolve_llm\` or \`ask_credential\`. - Valid provider tool keys are complete provider tool IDs documented in the Tool Guidance section. - \`providerTools\` keys must be complete provider tool IDs from the valid key list. diff --git a/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts index 61275154c15..9e31930dec2 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 @@ -50,7 +50,7 @@ export function getConfigRulesSection(): string { #### Agent Config Rules - \`model\` must be "provider/model-name". -- \`credential\` must be the id returned by \`resolve_llm\` or \`ask_llm\`. +- \`credential\` must be the id returned by \`resolve_llm\`. - Fresh agents must include \`memory: { "enabled": true, "storage": "n8n" }\` unless the user explicitly asks to disable memory. @@ -58,7 +58,7 @@ export function getConfigRulesSection(): string { - \`memory.episodicMemory\` requires \`ask_embedding_credential\` with \`credentialType: "openAiApi"\`; use its returned \`credentialId\` value. - Memory worker model fields use \`{ "model": "provider/model-name", "credential": "" }\`; - use only credential IDs returned by \`resolve_llm\`, \`ask_llm\`, or \`ask_credential\`. + use only credential IDs returned by \`resolve_llm\` or \`ask_credential\`. - Sub-agent configuration lives at top level under \`subAgents\`. Load \`agent-builder-sub-agents\` before adding refs or changing \`subAgents.maxChildren\`. 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 e4796a75238..3092b776a39 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 @@ -1,7 +1,7 @@ export function getLlmSelectionPrompt(modelRecommendationsSection: string | null): string { const recommendationGuidance = modelRecommendationsSection ? `\n\n${modelRecommendationsSection}` - : '\n\nNo Recommended LLM models section is available; do not recommend or name current, best, latest, or fallback model IDs from memory. Call ask_llm when the user needs model guidance or choice.'; + : '\n\nNo Recommended LLM models section is available; do not recommend or name current, best, latest, or fallback model IDs from memory. Ask via `ask_questions` when the user needs model guidance or choice.'; return `\ ## LLM Selection Guidance @@ -12,17 +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 call \`ask_llm\`. +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\`. -3. If the user asks to pick, change, confirm, or configure a model or main credential, call \`ask_llm\`; do not ask in prose. -4. If \`resolve_llm\` reports missing or ambiguous credentials/provider, call \`ask_llm\`. -5. If \`resolve_llm\` reports \`unknown_model\`, retry with a plausible returned model value or call \`ask_llm\`. +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. +5. If \`resolve_llm\` reports \`unknown_model\`, retry with a plausible returned model value or ask via \`ask_questions\`. ### Rules - Fresh agents need a resolved \`model\` and \`credential\` before config is written. - Explicit provider/model requests go to \`resolve_llm\` first. -- If the user asks to pick, change, confirm, or configure a model or main credential, call \`ask_llm\`; do not ask in prose. +- If the user asks to pick, change, confirm, or configure a model or main credential, ask via \`ask_questions\`; do not ask in prose. - If \`resolve_llm\` succeeds, persist \`model = "{provider}/{model}"\` and \`credential = credentialId\`. - Only OpenAI and Anthropic models support native web search. Use native web search by default for those providers only, and only for @@ -39,15 +39,15 @@ Use this to resolve the target agent's main \`model\` and \`credential\`. \`provider: "searxng"\`. - If the user explicitly asks for Brave or SearXNG, keep that provider even when the selected model also supports native search. -- If \`resolve_llm\` reports missing or ambiguous credentials/provider, call \`ask_llm\`. -- If it reports \`unknown_model\`, retry with a plausible returned model value or call \`ask_llm\`. +- If \`resolve_llm\` reports missing or ambiguous credentials/provider, ask via \`ask_questions\` then retry \`resolve_llm\`. +- If it reports \`unknown_model\`, retry with a plausible returned model value or ask via \`ask_questions\`. - For "Anthropic via OpenRouter", pass \`provider: "openrouter"\`; if the user names a routed model, pass the routed id without adding another provider prefix. - Prefer a provider the user already has credentials for when choosing from recommendations. - Never copy main LLM credential IDs from \`list_credentials\`. ### Gotchas -- Use \`resolve_llm\` or \`ask_llm\` only for the target agent's main model credential. +- Use \`resolve_llm\` only for the target agent's main model credential. - Use \`ask_credential\` for node tools, integrations, and Episodic Memory. - For OpenRouter, \`provider\` is \`"openrouter"\`; the model can be a routed id such as \`anthropic/...\`. - Model changes must not silently replace existing Brave or SearXNG web search with native search. @@ -56,6 +56,6 @@ Use this to resolve the target agent's main \`model\` and \`credential\`. ### Verify - The persisted \`model\` is in \`provider/model\` form. -- The persisted \`credential\` came from \`resolve_llm\` or \`ask_llm\`. +- The persisted \`credential\` came from \`resolve_llm\`. - Existing Brave or SearXNG \`config.webSearch\` is preserved on model changes unless the user explicitly requested a web-search method change.${recommendationGuidance}`; } 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 12c1a2ef0af..6571142bb49 100644 --- a/packages/cli/src/modules/agents/builder/prompts/memory.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/memory.prompt.ts @@ -31,7 +31,7 @@ separate user-facing memory product. - Supported observational memory tuning fields: \`enabled\`, \`observerModel\`, \`reflectorModel\`, \`observerThresholdTokens\`, \`reflectorThresholdTokens\`, \`renderTokenBudget\`, \`observationLogTailLimit\`, and \`lockTtlMs\`. - Memory worker model fields must use object shape: \`{ "model": "provider/model-name", "credential": "" }\`. - Only set \`observerModel\`, \`reflectorModel\`, \`extractorModel\`, or \`episodicMemory.reflectorModel\` when the user explicitly asks to use a specific model for memory work. -- Use only credential IDs returned by \`resolve_llm\`, \`ask_llm\`, or \`ask_credential\` for memory worker model fields. Do not invent IDs or copy a main-model credential unless one of those tools returned it for that worker model provider. +- Use only credential IDs returned by \`resolve_llm\` or \`ask_credential\` for memory worker model fields. Do not invent IDs or copy a main-model credential unless one of those tools returned it for that worker model provider. ### Episodic Memory diff --git a/packages/cli/src/modules/agents/instance-ai-builder-delegate.adapter.ts b/packages/cli/src/modules/agents/instance-ai-builder-delegate.adapter.ts index 8bcce05e135..b982a4583e2 100644 --- a/packages/cli/src/modules/agents/instance-ai-builder-delegate.adapter.ts +++ b/packages/cli/src/modules/agents/instance-ai-builder-delegate.adapter.ts @@ -1,5 +1,4 @@ import type { CredentialProvider, StreamChunk } from '@n8n/agents'; -import { ASK_LLM_TOOL_NAME } from '@n8n/api-types'; import type { User } from '@n8n/db'; import { Service } from '@n8n/di'; import type { @@ -18,26 +17,17 @@ import type { BuilderSessionOptions } from './builder/agents-builder.service'; import { getSuspendedToolCalls } from './utils/messages-envelope'; /** - * Standard builder tool names to omit for instance-AI sub-agent sessions. - * `ask_llm` has no card UI in instance-AI chat (a bounce workaround exists in - * instance-ai's build-agent tool instead), so it must never be offered here. - */ -export const BUILDER_EXCLUDED_TOOL_NAMES: string[] = [ASK_LLM_TOOL_NAME]; - -/** - * Prompt addendum for sub-agent runs; exported for tests. `ask_questions` and - * `configure_channel` are now part of the builder's own standard toolset and - * carry their own usage guidance in the builder prompts/skills — only the - * rules that are genuinely specific to running inside instance AI's chat - * remain here. + * Prompt addendum for sub-agent runs; exported for tests. `ask_questions`, + * `resolve_llm`, and `configure_channel` are all part of the builder's own + * standard toolset and carry their own usage guidance in the builder + * prompts/skills — only the rule that is genuinely specific to running + * inside instance AI's chat remains here. */ export const INSTANCE_AI_BUILDER_ADDENDUM = `## Instance AI session rules You are running as a sub-agent inside n8n's instance AI chat; the user sees your questions as chat cards. -The agent preview link is not visible in this chat; describe outcomes in text instead of linking the preview. - -\`ask_llm\` is NOT available in this chat. For model selection: ask via \`ask_questions\`, then call \`resolve_llm\` with the choice to resolve the credential.`; +The agent preview link is not visible in this chat; describe outcomes in text instead of linking the preview.`; function isTextDeltaChunk( chunk: StreamChunk, @@ -90,16 +80,14 @@ export class InstanceAiBuilderDelegateAdapterService { ) {} /** - * Prompt rules that only apply to the sub-agent surface: `ask_llm` (no - * card UI in this chat) is excluded from the standard tool set. - * `ask_questions` and `configure_channel` are now part of the builder's + * Prompt rules that only apply to the sub-agent surface. `ask_questions`, + * `resolve_llm`, and `configure_channel` are all part of the builder's * own standard toolset — no per-session tool injection needed anymore. */ private buildSubAgentSession(session: BuilderDelegateSession): BuilderSessionOptions { return { threadId: session.threadId, instructionsAddendum: INSTANCE_AI_BUILDER_ADDENDUM, - excludeTools: BUILDER_EXCLUDED_TOOL_NAMES, modelConfig: session.modelConfig, }; } diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 8ba4e4bbc62..115d6d120de 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -4822,7 +4822,6 @@ "agents.modelSelector.freeCredits.label": "Use free OpenAI credits", "agents.modelSelector.freeCredits.badge": "free credits", "agents.modelSelector.freeCredits.description": "Get {credits} free OpenAI API credits. Try it with gpt-5-mini.", - "agents.askLlm.chooseModel": "Choose a model", "agents.builder.unconfigured.title": "Set up the agent builder", "agents.builder.unconfigured.description.admin": "Choose a provider and credential so the agent builder can help you design agents.", "agents.builder.unconfigured.description.nonAdmin": "Ask an instance admin to configure the agent builder before you can build agents here.", diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChatPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChatPanel.test.ts index 1e3f594a3d3..424a9430c9d 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChatPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChatPanel.test.ts @@ -3,7 +3,6 @@ import { flushPromises, mount } from '@vue/test-utils'; import { computed, h, ref } from 'vue'; import { ASK_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, type InteractiveToolName, } from '@n8n/api-types'; @@ -122,31 +121,24 @@ describe('AgentChatPanel', () => { questions: [{ id: 'q1', question: 'Pick one', type: 'single', options: ['slack'] }], }, } - : toolName === ASK_LLM_TOOL_NAME - ? { - toolName: ASK_LLM_TOOL_NAME, - toolCallId: 'tc-1', - runId: 'run-1', - input: { purpose: 'Choose a model' }, - } - : { - toolName: ASK_CREDENTIAL_TOOL_NAME, - toolCallId: 'tc-1', - runId: 'run-1', - input: { - requestId: 'req-1', - message: 'Choose Slack credentials', - severity: 'info', - credentialRequests: [ - { - credentialType: 'slackApi', - reason: 'Choose Slack credentials', - existingCredentials: [], - }, - ], - credentialFlow: { stage: 'generic' }, - }, + : { + toolName: ASK_CREDENTIAL_TOOL_NAME, + toolCallId: 'tc-1', + runId: 'run-1', + input: { + requestId: 'req-1', + message: 'Choose Slack credentials', + severity: 'info', + credentialRequests: [ + { + credentialType: 'slackApi', + reason: 'Choose Slack credentials', + existingCredentials: [], + }, + ], + credentialFlow: { stage: 'generic' }, }, + }, }; } @@ -392,7 +384,7 @@ describe('AgentChatPanel', () => { expect(chatInput.props('placeholder')).toBe('agents.chat.input.placeholder'); }); - it.each([ASK_LLM_TOOL_NAME, ASK_CREDENTIAL_TOOL_NAME])( + it.each([ASK_QUESTIONS_TOOL_NAME, ASK_CREDENTIAL_TOOL_NAME])( 'enables chat input while %s is unresolved (cancel-and-steer mode)', (toolName) => { messagesMock.value = [openInteractiveMessage(toolName)]; diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AskLlmCard.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AskLlmCard.test.ts deleted file mode 100644 index a70e46f9eda..00000000000 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AskLlmCard.test.ts +++ /dev/null @@ -1,181 +0,0 @@ -/* eslint-disable import-x/no-extraneous-dependencies, @typescript-eslint/no-explicit-any -- test-only */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { mount, flushPromises } from '@vue/test-utils'; -import { ref } from 'vue'; - -const selectorState = vi.hoisted(() => ({ - selection: { provider: 'anthropic', model: 'claude-sonnet-4-5' }, -})); - -const credentialsByProvider = ref | undefined>({ anthropic: 'cred-1' }); -const selectCredential = vi.fn(); -const ensureLoaded = vi.fn(); -const getModelsForPicker = vi.fn(() => ({ - anthropic: { models: [] }, -})); -const isLoading = ref(false); -const personalProject = ref<{ id: string } | null>({ id: 'p1' }); -const allCredentials = ref>([ - { id: 'cred-1', name: 'My Anthropic' }, -]); -const currentUserId = ref('user-1'); - -vi.mock('@/features/settings/users/users.store', () => ({ - useUsersStore: () => ({ - get currentUserId() { - return currentUserId.value; - }, - }), -})); - -vi.mock('@/features/credentials/credentials.store', () => ({ - useCredentialsStore: () => ({ - get allCredentials() { - return allCredentials.value; - }, - }), -})); - -vi.mock('@/features/collaboration/projects/projects.store', () => ({ - useProjectsStore: () => ({ - get personalProject() { - return personalProject.value; - }, - }), -})); - -vi.mock('vue-router', async (importOriginal) => { - const actual = (await importOriginal()) as Record; - return { ...actual, useRoute: () => ({ params: {} }) }; -}); - -vi.mock('../composables/useModelCatalog', () => ({ - useModelCatalog: () => ({ - ensureLoaded, - getModelsForPicker, - isLoading, - }), -})); - -vi.mock('../composables/useAgentModelCredentials', () => ({ - useAgentModelCredentials: () => ({ credentialsByProvider, selectCredential }), -})); - -vi.mock('../components/AgentModelSelector.vue', () => ({ - default: { - name: 'AgentModelSelector', - props: [ - 'selectedModel', - 'credentials', - 'modelsByProvider', - 'isLoading', - 'warnMissingCredentials', - ], - emits: ['change', 'select-credential'], - setup(_props: unknown, { emit }: { emit: (event: string, payload: unknown) => void }) { - return { - selectModel: () => emit('change', selectorState.selection), - }; - }, - template: '
', - }, -})); - -import AskLlmCard from '../components/interactive/AskLlmCard.vue'; - -function mountCard(props: Record = {}) { - return mount(AskLlmCard, { - props, - global: { - stubs: { - N8nText: { template: '', props: ['size', 'bold', 'color', 'tag'] }, - N8nIcon: { template: '', props: ['icon', 'size', 'color'] }, - }, - }, - }); -} - -beforeEach(() => { - vi.clearAllMocks(); - selectorState.selection = { provider: 'anthropic', model: 'claude-sonnet-4-5' }; - credentialsByProvider.value = { anthropic: 'cred-1' }; - personalProject.value = { id: 'p1' }; - allCredentials.value = [{ id: 'cred-1', name: 'My Anthropic' }]; -}); - -describe('AskLlmCard', () => { - it('renders the purpose text', async () => { - const wrapper = mountCard({ purpose: 'Pick a main model' }); - await flushPromises(); - expect(wrapper.text()).toContain('Pick a main model'); - }); - - it('falls back to the default heading when no purpose is supplied', async () => { - const wrapper = mountCard(); - await flushPromises(); - expect(wrapper.text()).toContain('Choose a model'); - }); - - it('fetches the model catalog on mount', async () => { - mountCard(); - await flushPromises(); - expect(ensureLoaded).toHaveBeenCalledWith('p1'); - }); - - it('emits a complete resume payload when the model selector emits a change', async () => { - const wrapper = mountCard(); - await flushPromises(); - - await wrapper.find('[data-testid="model-selector"]').trigger('click'); - - const emitted = wrapper.emitted('submit') as unknown[][]; - expect(emitted).toBeTruthy(); - expect(emitted[0][0]).toEqual({ - provider: 'anthropic', - model: 'claude-sonnet-4-5', - credentialId: 'cred-1', - credentialName: 'My Anthropic', - }); - }); - - it('does NOT emit when disabled — guards against accidental commits in resolved state', async () => { - const wrapper = mountCard({ disabled: true }); - await flushPromises(); - - const selector = wrapper.find('[data-testid="model-selector"]'); - if (selector.exists()) await selector.trigger('click'); - - expect(wrapper.emitted('submit')).toBeFalsy(); - }); - - it('renders the resolved provider/model and credential name when given resolvedValue', async () => { - const wrapper = mountCard({ - disabled: true, - resolvedValue: { - provider: 'anthropic', - model: 'claude-sonnet-4-5', - credentialId: 'cred-1', - credentialName: 'My Anthropic', - }, - }); - await flushPromises(); - - const text = wrapper.text(); - expect(text).toContain('anthropic/claude-sonnet-4-5'); - expect(text).toContain('My Anthropic'); - }); - - it('strips the "models/" prefix from Google model ids before emitting', async () => { - selectorState.selection = { provider: 'google', model: 'models/gemini-2.5-pro' }; - credentialsByProvider.value = { google: 'cred-g' }; - allCredentials.value = [{ id: 'cred-g', name: 'My Google' }]; - - const wrapper = mountCard(); - await flushPromises(); - - await wrapper.find('[data-testid="model-selector"]').trigger('click'); - - const emitted = wrapper.emitted('submit') as unknown[][]; - expect((emitted[0][0] as { model: string }).model).toBe('gemini-2.5-pro'); - }); -}); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts index 65fe96d53f2..ce86c4cc2d3 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect } from 'vitest'; import { ASK_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, APPROVAL_TOOL_NAME, N8N_CHAT_ACTION_TOOL_NAME, @@ -28,46 +27,18 @@ function credentialSuspendInput(credentialType: string, reason: string) { }; } +/** A full `questionsSuspendPayloadSchema`-shaped input, for fixtures that construct `InteractivePayload` literals directly. */ +function questionsSuspendInput(question: string) { + return { + requestId: 'req-q1', + message: question, + severity: 'info' as const, + inputType: 'questions' as const, + questions: [{ id: 'q1', question, type: 'text' as const }], + }; +} + describe('rebuildInteractiveFromHistory', () => { - it('rebuilds an OPEN ask_llm card when output is missing', () => { - const result = rebuildInteractiveFromHistory({ - tool: ASK_LLM_TOOL_NAME, - toolCallId: 'call-1', - input: { purpose: 'pick a model' }, - state: 'suspended', - }); - - expect(result).toBeTruthy(); - expect(result?.toolName).toBe(ASK_LLM_TOOL_NAME); - expect(result?.resolvedAt).toBeUndefined(); - expect(result?.resolvedValue).toBeUndefined(); - // runId is the sidecar's responsibility — raw history doesn't carry it. - expect(result?.runId).toBeUndefined(); - }); - - it('rebuilds a RESOLVED ask_llm card when output is present', () => { - const result = rebuildInteractiveFromHistory({ - tool: ASK_LLM_TOOL_NAME, - toolCallId: 'call-1', - input: { purpose: 'pick a model' }, - output: { - provider: 'anthropic', - model: 'claude-sonnet-4', - credentialId: 'cred-1', - credentialName: 'My Anthropic', - }, - state: 'done', - }); - - expect(result?.resolvedAt).toBeGreaterThan(0); - expect(result?.resolvedValue).toEqual({ - provider: 'anthropic', - model: 'claude-sonnet-4', - credentialId: 'cred-1', - credentialName: 'My Anthropic', - }); - }); - it('rebuilds an ask_credential card with skipped resolved value', () => { const result = rebuildInteractiveFromHistory({ tool: ASK_CREDENTIAL_TOOL_NAME, @@ -149,9 +120,11 @@ describe('convertDbMessages — interactive turn synthesis', () => { content: [ { type: 'tool-call', - toolName: ASK_LLM_TOOL_NAME, - toolCallId: 'call-llm-1', - input: { purpose: 'main' }, + toolName: ASK_QUESTIONS_TOOL_NAME, + toolCallId: 'call-q-1', + input: { + questions: [{ id: 'q1', question: 'Which model?', type: 'text' }], + }, state: 'pending', }, ], @@ -163,7 +136,7 @@ describe('convertDbMessages — interactive turn synthesis', () => { const assistant = chat[1]; expect(assistant.role).toBe('assistant'); expect(assistant.status).toBe('awaitingUser'); - expect(assistant.interactive?.toolName).toBe(ASK_LLM_TOOL_NAME); + expect(assistant.interactive?.toolName).toBe(ASK_QUESTIONS_TOOL_NAME); expect(assistant.interactive?.resolvedAt).toBeUndefined(); expect(assistant.toolCalls?.[0].state).toBe('suspended'); }); @@ -470,11 +443,11 @@ describe('isGroupable', () => { id: 'm1', role: 'assistant', content: '', - toolCalls: [{ tool: ASK_LLM_TOOL_NAME, toolCallId: 'c1', state: 'suspended' }], + toolCalls: [{ tool: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c1', state: 'suspended' }], interactive: { - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c1', - input: {}, + input: questionsSuspendInput('Which model?'), }, status: 'awaitingUser', }); @@ -496,22 +469,20 @@ describe('isGroupable', () => { describe('buildDisplayGroups — interactive payloads', () => { it('collects interactive payloads from each grouped message into the toolRun group', () => { const groups = buildDisplayGroups([ - // First grouped turn: a resolved ask_llm card + // First grouped turn: a resolved ask_questions card { id: 'm1', role: 'assistant', content: '', - toolCalls: [{ tool: ASK_LLM_TOOL_NAME, toolCallId: 'c1', state: 'done' }], + toolCalls: [{ tool: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c1', state: 'done' }], interactive: { - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c1', - input: {}, + input: questionsSuspendInput('Which model?'), resolvedAt: 1, resolvedValue: { - provider: 'a', - model: 'b', - credentialId: 'x', - credentialName: 'y', + answered: true, + answers: [{ questionId: 'q1', selectedOptions: ['gpt-4'] }], }, }, status: 'success', @@ -545,7 +516,7 @@ describe('buildDisplayGroups — interactive payloads', () => { if (grouped.kind !== 'toolRun') return; expect(grouped.toolCalls).toHaveLength(3); expect(grouped.interactives).toHaveLength(2); - expect(grouped.interactives[0].toolName).toBe(ASK_LLM_TOOL_NAME); + expect(grouped.interactives[0].toolName).toBe(ASK_QUESTIONS_TOOL_NAME); expect(grouped.interactives[0].resolvedAt).toBeDefined(); expect(grouped.interactives[1].toolName).toBe(ASK_CREDENTIAL_TOOL_NAME); expect(grouped.interactives[1].resolvedAt).toBeUndefined(); @@ -665,17 +636,15 @@ describe('applyOpenSuspensions', () => { id: 'm2', role: 'assistant', content: '', - toolCalls: [{ tool: ASK_LLM_TOOL_NAME, toolCallId: 'c-resolved', state: 'done' }], + toolCalls: [{ tool: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c-resolved', state: 'done' }], interactive: { - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c-resolved', - input: { purpose: 'main' }, + input: questionsSuspendInput('Which model?'), resolvedAt: 1, resolvedValue: { - provider: 'a', - model: 'b', - credentialId: 'x', - credentialName: 'y', + answered: true, + answers: [{ questionId: 'q1', selectedOptions: ['gpt-4'] }], }, }, status: 'success', @@ -696,9 +665,9 @@ describe('applyOpenSuspensions', () => { role: 'assistant', content: '', interactive: { - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c1', - input: { purpose: 'main' }, + input: questionsSuspendInput('Which model?'), }, }, ]; @@ -714,9 +683,9 @@ describe('applyOpenSuspensions', () => { role: 'assistant', content: '', interactive: { - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'c1', - input: { purpose: 'main' }, + input: questionsSuspendInput('Which model?'), }, }, ]; diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/interactive-summary.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/interactive-summary.test.ts index 6f79104a902..07124a3bdc3 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/interactive-summary.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/interactive-summary.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from 'vitest'; import { ASK_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME, N8N_CHAT_ACTION_TOOL_NAME, @@ -26,7 +25,6 @@ describe('summariseInteractiveOutput', () => { 'returns undefined for non-object output (%p)', (value) => { expect(summariseInteractiveOutput(ASK_CREDENTIAL_TOOL_NAME, value)).toBeUndefined(); - expect(summariseInteractiveOutput(ASK_LLM_TOOL_NAME, value)).toBeUndefined(); expect(summariseInteractiveOutput(ASK_QUESTIONS_TOOL_NAME, value)).toBeUndefined(); }, ); @@ -63,17 +61,6 @@ describe('summariseInteractiveOutput', () => { expect(summariseInteractiveOutput(ASK_CREDENTIAL_TOOL_NAME, { skipped: true })).toBe('Skipped'); }); - it('renders ask_llm provider/model + credential', () => { - expect( - summariseInteractiveOutput(ASK_LLM_TOOL_NAME, { - provider: 'anthropic', - model: 'claude-sonnet-4-6', - credentialId: 'c1', - credentialName: 'My Anthropic', - }), - ).toBe('anthropic/claude-sonnet-4-6 · My Anthropic'); - }); - it('renders configure_channel connected/skipped', () => { expect(summariseInteractiveOutput(CONFIGURE_CHANNEL_TOOL_NAME, { connected: true })).toBe( 'Connected', diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/toolDisplayName.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/toolDisplayName.test.ts index b070c80b228..0a329128f3e 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/toolDisplayName.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/toolDisplayName.test.ts @@ -9,7 +9,7 @@ import { describe('formatToolNameForDisplay', () => { it('formats snake_case builder tool names as readable labels', () => { expect(formatToolNameForDisplay('create_skill')).toBe('Create skill'); - expect(formatToolNameForDisplay('ask_llm')).toBe('Ask LLM'); + expect(formatToolNameForDisplay('resolve_llm')).toBe('Resolve LLM'); expect(formatToolNameForDisplay('build_custom_tool')).toBe('Build custom tool'); expect(formatToolNameForDisplay('update_memory')).toBe('Update memory'); }); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts index 3b7ae301c1c..af3ac486caf 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts @@ -4,7 +4,6 @@ import { ref, nextTick } from 'vue'; import { APPROVAL_TOOL_NAME, ASK_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, N8N_CHAT_ACTION_TOOL_NAME, type AgentSseEvent, @@ -79,15 +78,15 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { vi.restoreAllMocks(); }); - it('renders an interactive ask_llm card and stamps the runId from the suspended event', async () => { + it('renders an interactive ask_questions card and stamps the runId from the suspended event', async () => { const events: AgentSseEvent[] = [ { type: 'tool-call-suspended', payload: { toolCallId: 'tc-1', runId: 'run-42', - toolName: ASK_LLM_TOOL_NAME, - input: { purpose: 'main model' }, + toolName: ASK_QUESTIONS_TOOL_NAME, + input: { questions: [{ question: 'Which model?', type: 'text' }] }, }, }, { type: 'done' }, @@ -104,7 +103,7 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { expect(assistant.status).toBe('awaitingUser'); expect(assistant.toolCalls).toHaveLength(1); expect(assistant.toolCalls?.[0].state).toBe('suspended'); - expect(assistant.interactive?.toolName).toBe(ASK_LLM_TOOL_NAME); + expect(assistant.interactive?.toolName).toBe(ASK_QUESTIONS_TOOL_NAME); expect(assistant.interactive?.runId).toBe('run-42'); expect(assistant.interactive?.resolvedValue).toBeUndefined(); expect(assistant.interactive?.resolvedAt).toBeUndefined(); @@ -160,16 +159,16 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { { type: 'tool-call', toolCallId: 'tc-1', - toolName: ASK_LLM_TOOL_NAME, - input: { purpose: 'main' }, + toolName: ASK_QUESTIONS_TOOL_NAME, + input: { questions: [{ question: 'Which model?', type: 'text' }] }, }, { type: 'tool-call-suspended', payload: { toolCallId: 'tc-1', runId: 'run-7', - toolName: ASK_LLM_TOOL_NAME, - input: { purpose: 'main' }, + toolName: ASK_QUESTIONS_TOOL_NAME, + input: { questions: [{ question: 'Which model?', type: 'text' }] }, }, }, { type: 'done' }, @@ -356,18 +355,18 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { // in place, not push a duplicate into a freshly-minted ChatMessage. const events: AgentSseEvent[] = [ { type: 'start-step' }, - { type: 'tool-input-start', toolCallId: 'tc-1', toolName: ASK_LLM_TOOL_NAME }, + { type: 'tool-input-start', toolCallId: 'tc-1', toolName: ASK_QUESTIONS_TOOL_NAME }, { type: 'tool-call', toolCallId: 'tc-1', - toolName: ASK_LLM_TOOL_NAME, - input: { purpose: 'main' }, + toolName: ASK_QUESTIONS_TOOL_NAME, + input: { questions: [{ question: 'Which model?', type: 'text' }] }, }, { type: 'finish-step' }, { type: 'tool-execution-start', toolCallId: 'tc-1', - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, startTime: 1_000, }, { @@ -375,8 +374,8 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { payload: { toolCallId: 'tc-1', runId: 'run-9', - toolName: ASK_LLM_TOOL_NAME, - input: { purpose: 'main' }, + toolName: ASK_QUESTIONS_TOOL_NAME, + input: { questions: [{ question: 'Which model?', type: 'text' }] }, }, }, { type: 'done' }, diff --git a/packages/frontend/editor-ui/src/features/agents/components/interactive/AskLlmCard.vue b/packages/frontend/editor-ui/src/features/agents/components/interactive/AskLlmCard.vue deleted file mode 100644 index 217c3d396b4..00000000000 --- a/packages/frontend/editor-ui/src/features/agents/components/interactive/AskLlmCard.vue +++ /dev/null @@ -1,150 +0,0 @@ - - - - - diff --git a/packages/frontend/editor-ui/src/features/agents/components/interactive/InteractiveCard.vue b/packages/frontend/editor-ui/src/features/agents/components/interactive/InteractiveCard.vue index de3815f30b6..32dd5ec7eee 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/interactive/InteractiveCard.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/interactive/InteractiveCard.vue @@ -4,7 +4,6 @@ import { APPROVAL_TOOL_NAME, ASK_CREDENTIAL_TOOL_NAME, ASK_EMBEDDING_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME, N8N_CHAT_ACTION_TOOL_NAME, @@ -13,15 +12,14 @@ import type { AgentsChatInteractionRenderer } from '@/features/ai/shared/agentsC import InteractionRenderer from '@/features/ai/shared/agentsChat/components/InteractionRenderer.vue'; import type { InteractivePayload } from '@/features/ai/shared/agentsChat/types'; import AskCredentialCard from './AskCredentialCard.vue'; -import AskLlmCard from './AskLlmCard.vue'; import AskQuestionsCard from './AskQuestionsCard.vue'; import ApprovalCard from './ApprovalCard.vue'; import ConfigureChannelCard from './ConfigureChannelCard.vue'; import N8nChatActionCard from './N8nChatActionCard.vue'; /** - * Single dispatch point for the interactive cards. `ask_llm`, `approval`, - * and `chat_action` still dispatch by `toolName` (their payload shape isn't + * Single dispatch point for the interactive cards. `approval` and + * `chat_action` still dispatch by `toolName` (their payload shape isn't * shared with any other surface). `ask_questions`, `ask_credential` / * `ask_embedding_credential`, and `configure_channel` MATCH by the PAYLOAD * FIELD that's unique to their suspend schema @@ -143,19 +141,6 @@ const interactiveRenderers = [ }; }, }, - { - key: 'ask_llm', - component: AskLlmCard, - matches: (payload) => payload.toolName === ASK_LLM_TOOL_NAME, - getProps: (payload, context) => { - if (payload.toolName !== ASK_LLM_TOOL_NAME) return {}; - return { - purpose: payload.input.purpose, - resolvedValue: payload.resolvedValue, - projectId: context?.projectId, - }; - }, - }, { key: 'chat_action', component: N8nChatActionCard, diff --git a/packages/frontend/editor-ui/src/features/agents/utils/model-string.ts b/packages/frontend/editor-ui/src/features/agents/utils/model-string.ts index 830836cee05..cb316c18b85 100644 --- a/packages/frontend/editor-ui/src/features/agents/utils/model-string.ts +++ b/packages/frontend/editor-ui/src/features/agents/utils/model-string.ts @@ -1,7 +1,7 @@ /** * Model identifier helpers. The canonical storage format is `"/"`. - * Centralised here because three callers (Agent panel, Advanced panel, AskLlm - * card) used to roll their own and drifted on naming + edge cases. + * Centralised here because multiple callers (Agent info panel, memory panel, + * sub-agents panel) used to roll their own and drifted on naming + edge cases. */ export interface ParsedModel { diff --git a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/__tests__/messageMappers.test.ts b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/__tests__/messageMappers.test.ts index de17e6dd6f1..69c85e92de3 100644 --- a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/__tests__/messageMappers.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/__tests__/messageMappers.test.ts @@ -1,9 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { - ASK_LLM_TOOL_NAME, - ASK_QUESTIONS_TOOL_NAME, - type AgentPersistedMessageDto, -} from '@n8n/api-types'; +import { ASK_QUESTIONS_TOOL_NAME, type AgentPersistedMessageDto } from '@n8n/api-types'; import { applyOpenSuspensions, @@ -20,9 +16,9 @@ describe('shared agents chat message mapping', () => { content: [ { type: 'tool-call', - toolName: ASK_LLM_TOOL_NAME, + toolName: ASK_QUESTIONS_TOOL_NAME, toolCallId: 'call-1', - input: { purpose: 'main model' }, + input: { questions: [{ question: 'Which model?', type: 'text' }] }, state: 'pending', }, ], @@ -32,7 +28,7 @@ describe('shared agents chat message mapping', () => { const chat = convertDbMessages(dbMessages); expect(chat[0].status).toBe('awaitingUser'); - expect(chat[0].interactive?.toolName).toBe(ASK_LLM_TOOL_NAME); + expect(chat[0].interactive?.toolName).toBe(ASK_QUESTIONS_TOOL_NAME); expect(chat[0].toolCalls?.[0].state).toBe('suspended'); }); diff --git a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/interactiveSummary.ts b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/interactiveSummary.ts index cfa81779634..b44b92abc83 100644 --- a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/interactiveSummary.ts +++ b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/interactiveSummary.ts @@ -1,11 +1,9 @@ import { ASK_CREDENTIAL_TOOL_NAME, ASK_EMBEDDING_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME, N8N_CHAT_ACTION_TOOL_NAME, - type AskLlmResume, } from '@n8n/api-types'; import { @@ -73,13 +71,6 @@ export function summariseInteractiveOutput( return undefined; } - if (toolName === ASK_LLM_TOOL_NAME) { - const resume = output as AskLlmResume; - if (!resume.provider || !resume.model) return undefined; - const slug = `${resume.provider}/${resume.model}`; - return resume.credentialName ? `${slug} · ${resume.credentialName}` : slug; - } - if (toolName === CONFIGURE_CHANNEL_TOOL_NAME) { if (typeof output.connected !== 'boolean') return undefined; return output.connected ? 'Connected' : 'Skipped'; diff --git a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/messageMappers.ts b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/messageMappers.ts index 7950f05f57e..74af8df66ef 100644 --- a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/messageMappers.ts +++ b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/messageMappers.ts @@ -1,14 +1,11 @@ import { ASK_CREDENTIAL_TOOL_NAME, ASK_EMBEDDING_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, APPROVAL_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME, N8N_CHAT_ACTION_TOOL_NAME, askCredentialInputSchema, - askLlmInputSchema, - askLlmResumeSchema, channelResumeSchema, channelSuspendPayloadSchema, credentialResumeSchema, @@ -49,7 +46,6 @@ import type { const INTERACTIVE_TOOL_NAMES = [ ASK_CREDENTIAL_TOOL_NAME, ASK_EMBEDDING_CREDENTIAL_TOOL_NAME, - ASK_LLM_TOOL_NAME, ASK_QUESTIONS_TOOL_NAME, CONFIGURE_CHANNEL_TOOL_NAME, ] as readonly InteractiveToolName[]; @@ -156,7 +152,7 @@ function isDeclinedToolOutput(value: unknown): boolean { // --------------------------------------------------------------------------- // // These three tools transform their resume payload into a distinct tool -// OUTPUT shape rather than echoing it back (unlike ask_llm). `tc.input` is +// OUTPUT shape rather than echoing it back. `tc.input` is // similarly dual-shaped: live suspensions overwrite it with the full suspend // payload (see `useAgentChatStream`'s `tool-call-suspended` handler), but // persisted history only ever carries the tool's original call args (the @@ -300,9 +296,8 @@ function parseAskEmbeddingCredentialOutput(value: unknown) { * reconstruct an `InteractivePayload` for it. The result is: * * - **resolved**: when `output` is present — `resolvedValue` is parsed from it - * via the matching zod schema. The output IS the user's resume payload (the - * tool handler returns `ctx.resumeData` after a resume) for `ask_llm`; the - * other interactive tools transform it, so `resolvedValue` is parsed + * via the matching zod schema. Interactive tools transform the resume + * payload into a distinct output shape, so `resolvedValue` is parsed * defensively (see the `parse*ResolvedValue` helpers above). * - **open**: when `output` is absent — the card renders as an active * awaiting-user prompt. Used when a refresh during a suspension restored the @@ -373,18 +368,6 @@ export function rebuildInteractiveFromHistory( }; } - if (tc.tool === ASK_LLM_TOOL_NAME) { - const input = askLlmInputSchema.safeParse(tc.input ?? {}); - if (!input.success) return undefined; - const resolved = tc.output !== undefined ? askLlmResumeSchema.safeParse(tc.output) : null; - return { - ...base, - toolName: ASK_LLM_TOOL_NAME, - input: input.data, - ...(resolved?.success && { resolvedValue: resolved.data }), - }; - } - if (tc.tool === CONFIGURE_CHANNEL_TOOL_NAME) { const input = buildChannelPayloadFromInput(tc.input, context); if (!input) return undefined; diff --git a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/types.ts b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/types.ts index f7bfe233a9d..3a78bd4fd17 100644 --- a/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/types.ts +++ b/packages/frontend/editor-ui/src/features/ai/shared/agentsChat/types.ts @@ -2,12 +2,9 @@ import { type APPROVAL_TOOL_NAME, type ASK_CREDENTIAL_TOOL_NAME, type ASK_EMBEDDING_CREDENTIAL_TOOL_NAME, - type ASK_LLM_TOOL_NAME, type ASK_QUESTIONS_TOOL_NAME, type CONFIGURE_CHANNEL_TOOL_NAME, type N8N_CHAT_ACTION_TOOL_NAME, - type AskLlmInput, - type AskLlmResume, type ChannelResumeData, type ChannelSuspendPayload, type CredentialResumeData, @@ -78,8 +75,8 @@ export interface ApprovalResume { /** * `ask_questions` / `ask_credential` / `configure_channel` each transform * the FE's resume payload into a distinct tool-output shape rather than - * echoing it back verbatim (unlike `ask_llm`). `resolvedValue` can therefore - * be EITHER shape depending on where it came from: + * echoing it back verbatim. `resolvedValue` can therefore be EITHER shape + * depending on where it came from: * * - the resume payload itself (optimistic UI update in `useAgentChatStream`'s * `resume()`, applied before the backend confirms), or @@ -113,11 +110,6 @@ export type InteractivePayload = input: ApprovalInput; resolvedValue?: ApprovalResume; }) - | (InteractivePayloadBase & { - toolName: typeof ASK_LLM_TOOL_NAME; - input: AskLlmInput; - resolvedValue?: AskLlmResume; - }) | (InteractivePayloadBase & { toolName: typeof ASK_QUESTIONS_TOOL_NAME; input: QuestionsSuspendPayload;