From f15fd87bab7dc835042ba4b46c450ebd364ecd88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Mon, 13 Oct 2025 15:39:42 +0200 Subject: [PATCH] fix(editor): Address MCP feedback (no-changelog) (#20595) --- .../cli/src/events/maps/relay.event-map.ts | 14 +++++ .../__tests__/mcp.settings.controller.test.ts | 40 ++++++------- .../cli/src/modules/mcp/mcp.event-relay.ts | 60 +++++++++++++++++++ packages/cli/src/modules/mcp/mcp.module.ts | 4 ++ .../modules/mcp/mcp.settings.controller.ts | 2 +- .../handlers/workflows/workflows.handler.ts | 14 +++++ .../cli/src/workflows/workflow.service.ts | 30 ++++++++++ .../frontend/@n8n/i18n/src/locales/en.json | 6 ++ .../src/components/MainHeader/MainHeader.vue | 20 +++++++ .../components/MainHeader/WorkflowDetails.vue | 11 ++++ .../editor-ui/src/components/WorkflowCard.vue | 17 +++++- .../WorkflowProductionChecklist.vue | 27 ++++++++- .../src/components/WorkflowSettings.vue | 20 +++---- .../src/composables/useWorkflowsCache.ts | 2 +- .../features/mcpAccess/SettingsMCPView.vue | 17 +----- .../components/MCPConnectionInstructions.vue | 29 ++++----- .../features/mcpAccess/composables/useMcp.ts | 20 +++++++ .../src/features/mcpAccess/mcp.constants.ts | 3 + .../src/features/mcpAccess/mcp.store.ts | 18 ++++++ .../editor-ui/src/views/WorkflowsView.vue | 12 +++- 20 files changed, 293 insertions(+), 73 deletions(-) create mode 100644 packages/cli/src/modules/mcp/mcp.event-relay.ts create mode 100644 packages/frontend/editor-ui/src/features/mcpAccess/composables/useMcp.ts diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index 7c61058c67e..6e75253005c 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -88,6 +88,20 @@ export type RelayEventMap = { publicApi: boolean; }; + 'workflow-activated': { + user: UserLike; + workflowId: string; + workflow: IWorkflowDb; + publicApi: boolean; + }; + + 'workflow-deactivated': { + user: UserLike; + workflowId: string; + workflow: IWorkflowDb; + publicApi: boolean; + }; + 'workflow-pre-execute': { executionId: string; data: IWorkflowExecutionDataProcess /* main process */ | IWorkflowBase /* worker */; diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts index bd3572af668..aed2a0a8a48 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts @@ -239,6 +239,25 @@ describe('McpSettingsController', () => { expect(workflowService.update).not.toHaveBeenCalled(); }); + test('allows disabling MCP for inactive workflows', async () => { + workflowFinderService.findWorkflowForUser.mockResolvedValue( + createWorkflow({ active: false }), + ); + workflowService.update.mockResolvedValue({ + id: workflowId, + settings: { saveManualExecutions: true, availableInMCP: false }, + versionId: 'client-version', + } as unknown as WorkflowEntity); + + const req = createReq({}, { user }); + + await controller.toggleWorkflowMCPAccess(req, mock(), workflowId, { + availableInMCP: false, + }); + + expect(workflowService.update).toHaveBeenCalledTimes(1); + }); + test('rejects enabling MCP without active webhook nodes', async () => { workflowFinderService.findWorkflowForUser.mockResolvedValue( createWorkflow({ @@ -296,26 +315,5 @@ describe('McpSettingsController', () => { versionId: 'updated-version-id', }); }); - - test('rejects disabling MCP for inactive workflows', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue( - createWorkflow({ active: false }), - ); - workflowService.update.mockResolvedValue({ - id: workflowId, - settings: { saveManualExecutions: true, availableInMCP: false }, - versionId: 'client-version', - } as unknown as WorkflowEntity); - - const req = createReq({}, { user }); - - await expect( - controller.toggleWorkflowMCPAccess(req, mock(), workflowId, { - availableInMCP: false, - }), - ).rejects.toThrow(new BadRequestError('MCP access can only be set for active workflows')); - - expect(workflowService.update).not.toHaveBeenCalled(); - }); }); }); diff --git a/packages/cli/src/modules/mcp/mcp.event-relay.ts b/packages/cli/src/modules/mcp/mcp.event-relay.ts new file mode 100644 index 00000000000..68d40d38e1d --- /dev/null +++ b/packages/cli/src/modules/mcp/mcp.event-relay.ts @@ -0,0 +1,60 @@ +import { Logger } from '@n8n/backend-common'; +import { WorkflowRepository } from '@n8n/db'; +import { Service } from '@n8n/di'; + +import { EventService } from '@/events/event.service'; +import { EventRelay } from '@/events/relays/event-relay'; +import type { RelayEventMap } from '@/events/maps/relay.event-map'; + +/** + * Event relay for MCP module to handle workflow events + */ +@Service() +export class McpEventRelay extends EventRelay { + constructor( + eventService: EventService, + private readonly workflowRepository: WorkflowRepository, + private readonly logger: Logger, + ) { + super(eventService); + } + + init() { + this.setupListeners({ + 'workflow-deactivated': async (event) => await this.onWorkflowDeactivated(event), + }); + } + + /** + * Handles workflow deactivated events. + * When a workflow is deactivated, automatically disables MCP access. + */ + private async onWorkflowDeactivated(event: RelayEventMap['workflow-deactivated']) { + const { workflow, workflowId } = event; + + // Only process if workflow has MCP access enabled + if (workflow.settings?.availableInMCP === true) { + try { + // Update the workflow settings to disable MCP access + const updatedSettings = { + ...workflow.settings, + availableInMCP: false, + }; + + await this.workflowRepository.update(workflowId, { + settings: updatedSettings, + }); + + this.logger.info('Disabled MCP access for deactivated workflow', { + workflowId, + workflowName: workflow.name, + }); + } catch (error) { + this.logger.error('Failed to disable MCP access for deactivated workflow', { + workflowId, + error: error instanceof Error ? error.message : String(error), + }); + } + } + } +} diff --git a/packages/cli/src/modules/mcp/mcp.module.ts b/packages/cli/src/modules/mcp/mcp.module.ts index b20427a24e3..a7e19112572 100644 --- a/packages/cli/src/modules/mcp/mcp.module.ts +++ b/packages/cli/src/modules/mcp/mcp.module.ts @@ -12,6 +12,10 @@ export class McpModule implements ModuleInterface { async init() { await import('./mcp.controller'); await import('./mcp.settings.controller'); + + // Initialize event relay to handle workflow deactivation + const { McpEventRelay } = await import('./mcp.event-relay'); + Container.get(McpEventRelay).init(); } /** diff --git a/packages/cli/src/modules/mcp/mcp.settings.controller.ts b/packages/cli/src/modules/mcp/mcp.settings.controller.ts index bfa61c212aa..e5677a1727e 100644 --- a/packages/cli/src/modules/mcp/mcp.settings.controller.ts +++ b/packages/cli/src/modules/mcp/mcp.settings.controller.ts @@ -78,7 +78,7 @@ export class McpSettingsController { ); } - if (!workflow.active) { + if (!workflow.active && dto.availableInMCP) { throw new BadRequestError('MCP access can only be set for active workflows'); } diff --git a/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts index 02ef3c8825a..cc9fbbf9b0a 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts @@ -368,6 +368,13 @@ export = { workflow.active = true; + Container.get(EventService).emit('workflow-activated', { + user: req.user, + workflowId: workflow.id, + workflow, + publicApi: true, + }); + return res.json(workflow); } @@ -402,6 +409,13 @@ export = { workflow.active = false; + Container.get(EventService).emit('workflow-deactivated', { + user: req.user, + workflowId: workflow.id, + workflow, + publicApi: true, + }); + return res.json(workflow); } diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index e54670198be..27fc84fba41 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -354,6 +354,28 @@ export class WorkflowService { publicApi: false, }); + // Check if workflow activation status changed + const wasActive = workflow.active; + const isNowActive = updatedWorkflow.active; + + if (isNowActive && !wasActive) { + // Workflow is being activated + this.eventService.emit('workflow-activated', { + user, + workflowId, + workflow: updatedWorkflow, + publicApi: false, + }); + } else if (!isNowActive && wasActive) { + // Workflow is being deactivated + this.eventService.emit('workflow-deactivated', { + user, + workflowId, + workflow: updatedWorkflow, + publicApi: false, + }); + } + if (updatedWorkflow.active) { // When the workflow is supposed to be active add it again try { @@ -370,6 +392,14 @@ export class WorkflowService { // Also set it in the returned data updatedWorkflow.active = false; + // Emit deactivation event since activation failed + this.eventService.emit('workflow-deactivated', { + user, + workflowId, + workflow: updatedWorkflow, + publicApi: false, + }); + let message; if (error instanceof NodeApiError) message = error.description; message = message ?? (error as Error).message; diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index ebb7dc57c59..4133ddf4546 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -1152,6 +1152,10 @@ "mainSidebar.whatsNew": "What’s New", "mainSidebar.whatsNew.fullChangelog": "Full changelog", "mcp.workflowNotEligable.description": "Only active, webhook-triggered workflows can be accessible through MCP", + "mcp.workflowDeactivated.title": "MCP Access Disabled", + "mcp.productionCheklist.title": "Enable MCP access", + "mcp.productionCheklist.description": "Allow MCP clients to access this workflow", + "mcp.workflowDeactivated.message": "MCP Access has been disabled for this workflow because it is deactivated.", "menuActions.duplicate": "Duplicate", "menuActions.download": "Download", "menuActions.push": "Push to Git", @@ -2694,6 +2698,7 @@ "workflowSettings.showError.saveSettings2.message": "The timeout is longer than allowed", "workflowSettings.showError.saveSettings2.title": "Problem saving settings", "workflowSettings.showError.saveSettings3.title": "Problem saving settings", + "workflowSettings.showError.fetchSettings.title": "Problem fetching settings", "workflowSettings.showMessage.saveSettings.title": "Workflow settings saved", "workflowSettings.timeoutAfter": "Timeout After", "workflowSettings.timeoutWorkflow": "Timeout Workflow", @@ -2804,6 +2809,7 @@ "workflows.empty.shared-with-me.link": "Back to Personal", "workflows.empty.readyToRunV2": "Try an AI workflow", "workflows.list.easyAI": "Test the power of AI in n8n with this simple AI Agent Workflow", + "workflows.list.error.fetching.one": "Error fetching workflow", "workflows.list.error.fetching": "Error fetching workflows", "workflows.shareModal.title": "Share '{name}'", "workflows.shareModal.title.static": "Shared with {projectName}", diff --git a/packages/frontend/editor-ui/src/components/MainHeader/MainHeader.vue b/packages/frontend/editor-ui/src/components/MainHeader/MainHeader.vue index ab7ff892755..f745adc7851 100644 --- a/packages/frontend/editor-ui/src/components/MainHeader/MainHeader.vue +++ b/packages/frontend/editor-ui/src/components/MainHeader/MainHeader.vue @@ -26,10 +26,12 @@ import GithubButton from 'vue-github-button'; import type { FolderShortInfo } from '@/features/folders/folders.types'; import { N8nIcon } from '@n8n/design-system'; +import { useToast } from '@/composables/useToast'; const router = useRouter(); const route = useRoute(); const locale = useI18n(); const pushConnection = usePushConnection({ router }); +const toast = useToast(); const ndvStore = useNDVStore(); const uiStore = useUIStore(); const sourceControlStore = useSourceControlStore(); @@ -244,6 +246,23 @@ async function navigateToEvaluationsView(openInNewTab: boolean) { function hideGithubButton() { githubButtonHidden.value = true; } + +async function onWorkflowDeactivated() { + if (settingsStore.isModuleActive('mcp') && workflow.value.settings?.availableInMCP) { + try { + // Fetch the updated workflow to get the latest settings after backend processing + const updatedWorkflow = await workflowsStore.fetchWorkflow(workflow.value.id); + workflowsStore.setWorkflow(updatedWorkflow); + toast.showToast({ + title: locale.baseText('mcp.workflowDeactivated.title'), + message: locale.baseText('mcp.workflowDeactivated.message'), + type: 'info', + }); + } catch (error) { + toast.showError(error, locale.baseText('workflowSettings.showError.fetchSettings.title')); + } + } +}