diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts index 4341cea10aa..567f13a7113 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/setup-workflow.service.test.ts @@ -7,6 +7,7 @@ import { applyNodeChanges, buildCompletedReport, createCredentialCache, + stripStaleCredentialsFromWorkflow, } from '../setup-workflow.service'; // --------------------------------------------------------------------------- @@ -357,6 +358,102 @@ describe('buildSetupRequests', () => { expect(context.credentialService.list).toHaveBeenCalledTimes(1); }); + it('does not generate credential request for HTTP Request with auth=none and stale node.credentials', async () => { + (context.nodeService as unknown as Record).getNodeCredentialTypes = jest + .fn() + .mockResolvedValue([]); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [ + { + name: 'httpHeaderAuth', + displayOptions: { show: { authentication: ['genericCredentialType'] } }, + }, + ], + }); + (context.credentialService.list as jest.Mock).mockResolvedValue([]); + + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { authentication: 'none', url: 'https://api.example.com' }, + credentials: { httpHeaderAuth: { id: 'old-cred', name: 'Stale Header Auth' } }, + }); + const result = await buildSetupRequests(context, node); + + expect(result.find((r) => r.credentialType === 'httpHeaderAuth')).toBeUndefined(); + }); + + it('fallback: displayOptions filtering takes priority over stale node.credentials', async () => { + // Remove getNodeCredentialTypes to force fallback path + delete (context.nodeService as unknown as Record).getNodeCredentialTypes; + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [ + { + name: 'httpHeaderAuth', + displayOptions: { show: { authentication: ['genericCredentialType'] } }, + }, + ], + }); + (context.credentialService.list as jest.Mock).mockResolvedValue([]); + + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { authentication: 'none', url: 'https://api.example.com' }, + credentials: { httpHeaderAuth: { id: 'old-cred', name: 'Stale Header Auth' } }, + }); + const result = await buildSetupRequests(context, node); + + expect(result.find((r) => r.credentialType === 'httpHeaderAuth')).toBeUndefined(); + }); + + it('fallback: node with assigned credentials matching description is still detected', async () => { + (context.nodeService as unknown as Record).getNodeCredentialTypes = jest + .fn() + .mockResolvedValue([]); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [{ name: 'slackApi' }], + }); + (context.credentialService.list as jest.Mock).mockResolvedValue([ + { id: 'cred-1', name: 'My Slack', updatedAt: '2025-01-01T00:00:00.000Z' }, + ]); + + const node = makeNode({ + credentials: { slackApi: { id: 'cred-1', name: 'My Slack' } }, + }); + const result = await buildSetupRequests(context, node); + + expect(result.find((r) => r.credentialType === 'slackApi')).toBeDefined(); + }); + + it('fallback: node.credentials with types not in description are excluded', async () => { + // Remove getNodeCredentialTypes to force fallback path + delete (context.nodeService as unknown as Record).getNodeCredentialTypes; + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [{ name: 'slackApi' }], + }); + (context.credentialService.list as jest.Mock).mockResolvedValue([ + { id: 'cred-1', name: 'My Slack', updatedAt: '2025-01-01T00:00:00.000Z' }, + ]); + + const node = makeNode({ + credentials: { + slackApi: { id: 'cred-1', name: 'My Slack' }, + httpHeaderAuth: { id: 'stale', name: 'Stale Auth' }, + }, + }); + const result = await buildSetupRequests(context, node); + + expect(result.find((r) => r.credentialType === 'slackApi')).toBeDefined(); + expect(result.find((r) => r.credentialType === 'httpHeaderAuth')).toBeUndefined(); + }); + it('treats placeholder values as parameter issues', async () => { (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ group: [], @@ -571,6 +668,101 @@ describe('applyNodeChanges', () => { expect(result.failed).toHaveLength(1); expect(result.failed[0].error).toContain('Failed to save workflow'); }); + + it('strips credentials not valid for the current parameters', async () => { + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { authentication: 'none', url: 'https://api.example.com' }, + credentials: { httpHeaderAuth: { id: 'stale', name: 'Stale Header Auth' } }, + }); + const wfJson = makeWorkflowJSON([node]); + (context.workflowService.getAsWorkflowJSON as jest.Mock).mockResolvedValue(wfJson); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [ + { + name: 'httpHeaderAuth', + displayOptions: { show: { authentication: ['genericCredentialType'] } }, + }, + ], + }); + (context.workflowService.updateFromWorkflowJSON as jest.Mock).mockResolvedValue(undefined); + + await applyNodeChanges(context, 'wf-1'); + + const calls = (context.workflowService.updateFromWorkflowJSON as jest.Mock).mock.calls as Array< + [string, WorkflowJSON] + >; + const savedJson = calls[0][1]; + const savedNode = savedJson.nodes.find((n) => n.name === 'HTTP Request'); + expect(savedNode?.credentials).toBeUndefined(); + }); + + it('preserves just-applied credentials even if description would exclude them', async () => { + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { authentication: 'none' }, + }); + const wfJson = makeWorkflowJSON([node]); + (context.workflowService.getAsWorkflowJSON as jest.Mock).mockResolvedValue(wfJson); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [], + }); + (context.credentialService.get as jest.Mock).mockResolvedValue({ + id: 'cred-1', + name: 'My Header Auth', + }); + (context.workflowService.updateFromWorkflowJSON as jest.Mock).mockResolvedValue(undefined); + + await applyNodeChanges(context, 'wf-1', { + 'HTTP Request': { httpHeaderAuth: 'cred-1' }, + }); + + const calls = (context.workflowService.updateFromWorkflowJSON as jest.Mock).mock.calls as Array< + [string, WorkflowJSON] + >; + const savedJson = calls[0][1]; + const savedNode = savedJson.nodes.find((n) => n.name === 'HTTP Request'); + expect(savedNode?.credentials).toEqual({ + httpHeaderAuth: { id: 'cred-1', name: 'My Header Auth' }, + }); + }); + + it('keeps credentials matching description displayOptions', async () => { + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { + authentication: 'genericCredentialType', + genericAuthType: 'httpHeaderAuth', + }, + credentials: { httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' } }, + }); + const wfJson = makeWorkflowJSON([node]); + (context.workflowService.getAsWorkflowJSON as jest.Mock).mockResolvedValue(wfJson); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [], + }); + (context.workflowService.updateFromWorkflowJSON as jest.Mock).mockResolvedValue(undefined); + + await applyNodeChanges(context, 'wf-1'); + + const calls = (context.workflowService.updateFromWorkflowJSON as jest.Mock).mock.calls as Array< + [string, WorkflowJSON] + >; + const savedJson = calls[0][1]; + const savedNode = savedJson.nodes.find((n) => n.name === 'HTTP Request'); + expect(savedNode?.credentials).toEqual({ + httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' }, + }); + }); }); // --------------------------------------------------------------------------- @@ -607,3 +799,117 @@ describe('buildCompletedReport', () => { expect(report).toHaveLength(0); }); }); + +// --------------------------------------------------------------------------- +// stripStaleCredentialsFromWorkflow +// --------------------------------------------------------------------------- + +describe('stripStaleCredentialsFromWorkflow', () => { + let context: InstanceAiContext; + + beforeEach(() => { + context = createMockContext(); + }); + + it('removes credential entries that no longer match the node parameters', async () => { + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { authentication: 'none', url: 'https://api.example.com' }, + credentials: { httpHeaderAuth: { id: 'stale', name: 'Stale Header Auth' } }, + }); + const wfJson = makeWorkflowJSON([node]); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [ + { + name: 'httpHeaderAuth', + displayOptions: { show: { authentication: ['genericCredentialType'] } }, + }, + ], + }); + + await stripStaleCredentialsFromWorkflow(context, wfJson); + + expect(wfJson.nodes[0].credentials).toBeUndefined(); + }); + + it('keeps credential entries that match the current parameters', async () => { + const node = makeNode({ + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { + authentication: 'genericCredentialType', + genericAuthType: 'httpHeaderAuth', + }, + credentials: { httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' } }, + }); + const wfJson = makeWorkflowJSON([node]); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [], + }); + + await stripStaleCredentialsFromWorkflow(context, wfJson); + + expect(wfJson.nodes[0].credentials).toEqual({ + httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' }, + }); + }); + + it('strips per-node — clean nodes are unaffected, stale nodes are scrubbed', async () => { + const cleanNode = makeNode({ + name: 'OpenRouter', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { + authentication: 'genericCredentialType', + genericAuthType: 'httpHeaderAuth', + }, + credentials: { httpHeaderAuth: { id: 'cred-1', name: 'OpenRouter Auth' } }, + }); + const staleNode = makeNode({ + name: 'Joke API', + id: 'node-2', + type: 'n8n-nodes-base.httpRequest', + typeVersion: 4.4, + parameters: { authentication: 'none', url: 'https://icanhazdadjoke.com/' }, + credentials: { httpHeaderAuth: { id: 'cred-1', name: 'OpenRouter Auth' } }, + }); + const wfJson = makeWorkflowJSON([cleanNode, staleNode]); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [ + { + name: 'httpHeaderAuth', + displayOptions: { show: { authentication: ['genericCredentialType'] } }, + }, + ], + }); + + await stripStaleCredentialsFromWorkflow(context, wfJson); + + expect(wfJson.nodes[0].credentials).toEqual({ + httpHeaderAuth: { id: 'cred-1', name: 'OpenRouter Auth' }, + }); + expect(wfJson.nodes[1].credentials).toBeUndefined(); + }); + + it('is a no-op for nodes without credentials', async () => { + const node = makeNode({ + parameters: { authentication: 'none' }, + }); + const wfJson = makeWorkflowJSON([node]); + (context.nodeService.getDescription as jest.Mock).mockResolvedValue({ + group: [], + credentials: [], + }); + + await stripStaleCredentialsFromWorkflow(context, wfJson); + + expect(wfJson.nodes[0].credentials).toBeUndefined(); + expect(context.nodeService.getDescription).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts index 97f07bc2fe7..ec74d0c6dd2 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts @@ -3,6 +3,7 @@ import { generateWorkflowCode, layoutWorkflowJSON } from '@n8n/workflow-sdk'; import { z } from 'zod'; import { buildCredentialMap, resolveCredentials } from './resolve-credentials'; +import { stripStaleCredentialsFromWorkflow } from './setup-workflow.service'; import { ensureWebhookIds } from './submit-workflow.tool'; import type { InstanceAiContext } from '../../types'; import { parseAndValidate, partitionWarnings } from '../../workflow-builder'; @@ -153,6 +154,12 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { const credentialMap = await buildCredentialMap(context.credentialService); await resolveCredentials(json, workflowId, context, credentialMap); + // Strip credential entries that are no longer valid for the current + // parameters. Resolution above (and the LLM itself) can re-emit stale + // references between turns; without this, setup analysis would surface + // a credential request for a node that no longer needs one. + await stripStaleCredentialsFromWorkflow(context, json); + // Ensure webhook nodes have a webhookId so n8n registers clean paths await ensureWebhookIds(json, workflowId, context); diff --git a/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts b/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts index dfa8611728a..af061c27f4a 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/setup-workflow.service.ts @@ -6,7 +6,7 @@ * state machine, and this logic is testable independently. */ import { findPlaceholderDetails } from '@n8n/utils'; -import type { IDataObject, NodeJSON, DisplayOptions } from '@n8n/workflow-sdk'; +import type { IDataObject, NodeJSON, DisplayOptions, WorkflowJSON } from '@n8n/workflow-sdk'; import { matchesDisplayOptions } from '@n8n/workflow-sdk'; import { nanoid } from 'nanoid'; @@ -31,6 +31,122 @@ export function createCredentialCache(): CredentialCache { // ── Node analysis ─────────────────────────────────────────────────────────── +/** + * Compute the set of credential types valid for a node given its current + * parameters. Mirrors the resolution in `buildSetupRequests`: consults the + * node service's dynamic resolver first, then falls back to the description's + * static credentials filtered by displayOptions, plus the dynamic types + * implied by `authentication: genericCredentialType | predefinedCredentialType`. + */ +export async function getValidCredentialTypes( + context: InstanceAiContext, + node: NodeJSON, +): Promise> { + const typeVersion = node.typeVersion ?? 1; + const parameters = (node.parameters as Record) ?? {}; + let nodeDesc: Awaited> | undefined; + try { + nodeDesc = await context.nodeService.getDescription(node.type, typeVersion); + } catch { + nodeDesc = undefined; + } + + const types = new Set(); + + if (context.nodeService.getNodeCredentialTypes) { + try { + const dynamic = await context.nodeService.getNodeCredentialTypes( + node.type, + typeVersion, + parameters, + node.credentials as Record | undefined, + ); + for (const t of dynamic) types.add(t); + } catch (error) { + // Falling through to description-based detection is safe, but the dynamic + // resolver isn't expected to throw — log so we can investigate if it does. + context.logger?.warn( + '[setup-workflow] getNodeCredentialTypes threw during credential validation', + { + nodeType: node.type, + typeVersion, + error: error instanceof Error ? error.message : String(error), + }, + ); + } + } + + if (nodeDesc?.credentials) { + for (const c of nodeDesc.credentials as Array<{ name?: string; displayOptions?: unknown }>) { + if (!c.name) continue; + if (!c.displayOptions) { + types.add(c.name); + continue; + } + if ( + matchesDisplayOptions( + { parameters, nodeVersion: typeVersion }, + c.displayOptions as DisplayOptions, + ) + ) { + types.add(c.name); + } + } + } + + const authentication = parameters.authentication; + if ( + authentication === 'genericCredentialType' && + typeof parameters.genericAuthType === 'string' + ) { + types.add(parameters.genericAuthType); + } else if ( + authentication === 'predefinedCredentialType' && + typeof parameters.nodeCredentialType === 'string' + ) { + types.add(parameters.nodeCredentialType); + } + + return types; +} + +/** + * Drop credential entries from a node whose type is no longer valid for the + * node's current parameters (e.g. an `httpHeaderAuth` left over from an earlier + * builder revision after `authentication` was switched to `none`). Mutates + * `node.credentials` in place. Types in `preserveTypes` are kept regardless — + * used by the apply path so a credential the user just assigned isn't stripped. + */ +export async function stripStaleCredentialsFromNode( + context: InstanceAiContext, + node: NodeJSON, + preserveTypes?: Set, +): Promise { + if (!node.credentials || Object.keys(node.credentials).length === 0) return; + const validTypes = await getValidCredentialTypes(context, node); + const cleaned: NonNullable = {}; + for (const [credType, value] of Object.entries(node.credentials)) { + if (validTypes.has(credType) || preserveTypes?.has(credType)) { + cleaned[credType] = value; + } + } + node.credentials = Object.keys(cleaned).length > 0 ? cleaned : undefined; +} + +/** + * Run {@link stripStaleCredentialsFromNode} over every node in a workflow. + * Intended to run after `resolveCredentials` in the builder save paths so the + * LLM can't persist stale credential references between turns. + */ +export async function stripStaleCredentialsFromWorkflow( + context: InstanceAiContext, + json: WorkflowJSON, +): Promise { + await Promise.all( + (json.nodes ?? []).map(async (node) => await stripStaleCredentialsFromNode(context, node)), + ); +} + /** * Build setup request(s) from a single WorkflowJSON node. * Detects credential types, auto-selects the most recent credential, @@ -120,27 +236,22 @@ export async function buildSetupRequests( } // Fallback: if dynamic detection returned nothing, check the node description's - // static credentials list and already-assigned credentials on the node. - // This catches cases where getNodeCredentialTypes fails silently (e.g. node - // lookup miss) or isn't available. - if (credentialTypes.length === 0) { - const nodeCredTypes = node.credentials ? Object.keys(node.credentials) : []; - if (nodeCredTypes.length > 0) { - credentialTypes = nodeCredTypes; - } else if (nodeDesc?.credentials) { - // Only include credentials whose displayOptions match the current parameters. - // Mirrors the evaluation in instance-ai.adapter.service.ts getNodeCredentialTypes(). - credentialTypes = nodeDesc.credentials - .filter((c: { name?: string; displayOptions?: unknown }) => { - if (!c.displayOptions) return true; - return matchesDisplayOptions( - { parameters, nodeVersion: typeVersion }, - c.displayOptions as DisplayOptions, - ); - }) - .map((c: { name?: string }) => c.name) - .filter((n): n is string => n !== undefined); - } + // static credentials list with displayOptions filtering. This catches cases where + // getNodeCredentialTypes fails silently (e.g. node lookup miss) or isn't available. + // We intentionally do NOT fall back to node.credentials here — stale credential + // entries (e.g. httpHeaderAuth left over from an earlier builder revision when + // authentication was later changed to 'none') must not generate setup requests. + if (credentialTypes.length === 0 && nodeDesc?.credentials) { + credentialTypes = nodeDesc.credentials + .filter((c: { name?: string; displayOptions?: unknown }) => { + if (!c.displayOptions) return true; + return matchesDisplayOptions( + { parameters, nodeVersion: typeVersion }, + c.displayOptions as DisplayOptions, + ); + }) + .map((c: { name?: string }) => c.name) + .filter((n): n is string => n !== undefined); } // Dynamic credential resolution for nodes that use genericCredentialType @@ -559,6 +670,12 @@ export async function applyNodeChanges( }); } } + + // Drop credential entries that are no longer valid for the node's current + // parameters. Entries just applied via credsMap are preserved so a user + // can assign an auxiliary credential without it being cleaned up. + const appliedTypes = new Set(credsMap ? Object.keys(credsMap) : []); + await stripStaleCredentialsFromNode(context, node, appliedTypes); } // Single save for all changes diff --git a/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts index 9cb14fb8de5..42f72ed5341 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts @@ -15,6 +15,7 @@ import { createHash, randomUUID } from 'node:crypto'; import { z } from 'zod'; import { resolveCredentials, type CredentialMap } from './resolve-credentials'; +import { stripStaleCredentialsFromWorkflow } from './setup-workflow.service'; import type { InstanceAiContext } from '../../types'; import type { ValidationWarning } from '../../workflow-builder'; import { partitionWarnings } from '../../workflow-builder'; @@ -304,6 +305,12 @@ export function createSubmitWorkflowTool( // Unresolved credentials are mocked via pinned data when available. const mockResult = await resolveCredentials(json, workflowId, context, credentialMap); + // Strip credential entries that are no longer valid for the current + // parameters. Resolution above (and the LLM itself) can re-emit stale + // references between turns; without this, setup analysis would surface + // a credential request for a node that no longer needs one. + await stripStaleCredentialsFromWorkflow(context, json); + // Ensure webhook nodes have a webhookId so n8n registers clean paths // (e.g., "{uuid}/dashboard" instead of "{workflowId}/{encodedNodeName}/dashboard"). // The SDK's toJSON() doesn't emit webhookId, so we inject it here. diff --git a/packages/@n8n/instance-ai/src/types.ts b/packages/@n8n/instance-ai/src/types.ts index cf99bb1f8ed..82ab40dc47b 100644 --- a/packages/@n8n/instance-ai/src/types.ts +++ b/packages/@n8n/instance-ai/src/types.ts @@ -532,6 +532,8 @@ export interface InstanceAiContext { * Used to register `parse-file` and supply data to the parser. */ currentUserAttachments?: InstanceAiAttachment[]; + /** Optional logger for diagnostics from domain tools. */ + logger?: Logger; } // ── Task storage ───────────────────────────────────────────────────────────── diff --git a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts index 6f2361668b6..d194ae5ee10 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts @@ -203,6 +203,7 @@ export class InstanceAiAdapterService { webResearchService: this.createWebResearchAdapter(user, searchProxyConfig), workspaceService: this.createWorkspaceAdapter(user), licenseHints: this.buildLicenseHints(), + logger: this.logger, }; } @@ -1689,7 +1690,7 @@ export class InstanceAiAdapterService { return filteredIssues; }, - getNodeCredentialTypes: async (nodeType, typeVersion, parameters, existingCredentials) => { + getNodeCredentialTypes: async (nodeType, typeVersion, parameters, _existingCredentials) => { const nodes = await getNodes(); const desc = findNodeByVersion(nodes, nodeType, typeVersion); if (!desc) return []; @@ -1767,13 +1768,6 @@ export class InstanceAiAdapterService { credentialTypes.add(parameters.nodeCredentialType as string); } - // 4. Already-assigned credentials - if (existingCredentials) { - for (const credType of Object.keys(existingCredentials)) { - credentialTypes.add(credType); - } - } - return Array.from(credentialTypes); },