diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts index 7d989c70904..7cb241a1f3e 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts @@ -1,7 +1,21 @@ -import { testDb } from '@n8n/backend-test-utils'; -import { ApiKeyRepository, type User } from '@n8n/db'; +import { + createTeamProject, + createWorkflow, + getPersonalProject, + linkUserToProject, + shareWorkflowWithProjects, + testDb, +} from '@n8n/backend-test-utils'; +import { + ApiKeyRepository, + FolderRepository, + WorkflowRepository, + type Project, + type User, +} from '@n8n/db'; import { Container } from '@n8n/di'; +import { createFolder } from '@test-integration/db/folders'; import { createMember, createOwner, createUser } from '@test-integration/db/users'; import { setupTestServer } from '@test-integration/utils'; @@ -184,3 +198,256 @@ describe('MCP API Key Edge Cases', () => { expect(storedApiKey.length).toBe(1); }); }); + +describe('PATCH /mcp/workflows/toggle-access', () => { + // Re-create users per test so we can truncate workflow/project/folder tables + // without leaving dangling references between cases. + let toggleOwner: User; + let toggleMember: User; + + const workflowRepository = () => Container.get(WorkflowRepository); + + const readAvailableInMCP = async (workflowId: string): Promise => { + const wf = await workflowRepository().findOneBy({ id: workflowId }); + return wf?.settings?.availableInMCP; + }; + + beforeEach(async () => { + await testDb.truncate([ + 'WorkflowEntity', + 'SharedWorkflow', + 'Folder', + 'ProjectRelation', + 'Project', + 'User', + ]); + toggleOwner = await createOwner(); + toggleMember = await createMember(); + }); + + test('rejects requests without a scope', async () => { + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: true }); + + expect(response.statusCode).toBeGreaterThanOrEqual(400); + expect(response.statusCode).toBeLessThan(500); + }); + + test('rejects requests with more than one scope', async () => { + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: true, workflowIds: ['wf-1'], projectId: 'project-1' }); + + expect(response.statusCode).toBeGreaterThanOrEqual(400); + expect(response.statusCode).toBeLessThan(500); + }); + + test('enables MCP access across an explicit workflow id list in a single transaction', async () => { + // Act as the member — the member lacks global workflow:update scope, so + // the finder service filters out workflows they cannot access. This is + // the behavior we want to exercise for the bulk path. + const ownedByMember = await createWorkflow( + { name: 'member-1', settings: { saveManualExecutions: true } }, + toggleMember, + ); + const ownedByMember2 = await createWorkflow({ name: 'member-2', settings: {} }, toggleMember); + const ownedByOwner = await createWorkflow({ name: 'owner-wf', settings: {} }, toggleOwner); + + const response = await testServer + .authAgentFor(toggleMember) + .patch('/mcp/workflows/toggle-access') + .send({ + availableInMCP: true, + workflowIds: [ownedByMember.id, ownedByMember2.id, ownedByOwner.id], + }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.updatedCount).toBe(2); + expect(response.body.data.updatedIds.sort()).toEqual( + [ownedByMember.id, ownedByMember2.id].sort(), + ); + expect(response.body.data.skippedCount).toBe(1); + + expect(await readAvailableInMCP(ownedByMember.id)).toBe(true); + expect(await readAvailableInMCP(ownedByMember2.id)).toBe(true); + // The unauthorized workflow remains untouched. + expect(await readAvailableInMCP(ownedByOwner.id)).toBeUndefined(); + + // Pre-existing settings on ownedByMember are preserved. + const refreshed = await workflowRepository().findOneBy({ id: ownedByMember.id }); + expect(refreshed?.settings?.saveManualExecutions).toBe(true); + }); + + test('skips archived workflows', async () => { + const live = await createWorkflow({ name: 'live', settings: {} }, toggleOwner); + const archived = await createWorkflow( + { name: 'archived', settings: {}, isArchived: true }, + toggleOwner, + ); + + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ + availableInMCP: true, + workflowIds: [live.id, archived.id], + }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.updatedIds).toEqual([live.id]); + expect(response.body.data.skippedCount).toBe(1); + expect(await readAvailableInMCP(archived.id)).toBeUndefined(); + }); + + test('scopes updates to a project id', async () => { + const project: Project = await createTeamProject('team-project'); + await linkUserToProject(toggleOwner, project, 'project:admin'); + + const projectWf1 = await createWorkflow({ name: 'project-wf-1', settings: {} }, project); + const projectWf2 = await createWorkflow({ name: 'project-wf-2', settings: {} }, project); + const unrelatedWf = await createWorkflow({ name: 'unrelated', settings: {} }, toggleOwner); + + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: true, projectId: project.id }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toEqual({ updatedCount: 2, skippedCount: 0, failedCount: 0 }); + expect(await readAvailableInMCP(projectWf1.id)).toBe(true); + expect(await readAvailableInMCP(projectWf2.id)).toBe(true); + expect(await readAvailableInMCP(unrelatedWf.id)).toBeUndefined(); + }); + + test('scopes updates to a folder id, including descendants', async () => { + const project = await getPersonalProject(toggleOwner); + const rootFolder = await createFolder(project, { name: 'root' }); + const childFolder = await createFolder(project, { + name: 'child', + parentFolder: await Container.get(FolderRepository).findOneByOrFail({ id: rootFolder.id }), + }); + + const workflowInRoot = await createWorkflow( + { name: 'wf-root', parentFolder: rootFolder }, + toggleOwner, + ); + const workflowInChild = await createWorkflow( + { name: 'wf-child', parentFolder: childFolder }, + toggleOwner, + ); + const workflowOutsideFolder = await createWorkflow({ name: 'wf-outside' }, toggleOwner); + + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: true, folderId: rootFolder.id }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toEqual({ updatedCount: 2, skippedCount: 0, failedCount: 0 }); + expect(await readAvailableInMCP(workflowInRoot.id)).toBe(true); + expect(await readAvailableInMCP(workflowInChild.id)).toBe(true); + expect(await readAvailableInMCP(workflowOutsideFolder.id)).toBeUndefined(); + }); + + test('counts a shared workflow exactly once regardless of its sharings', async () => { + // SharedWorkflow is keyed by (workflowId, projectId), so a workflow + // shared with multiple projects would previously surface as several + // rows from the finder. That inflated `skippedCount` even though the + // single underlying workflow was updated correctly. + const project = await getPersonalProject(toggleOwner); + const folder = await createFolder(project, { name: 'shared-container' }); + + const workflow = await createWorkflow( + { name: 'shared-wf', parentFolder: folder, settings: {} }, + toggleOwner, + ); + + // Extra sharings multiply the rows the finder sees for this workflow. + const extraProject1 = await createTeamProject('extra-1'); + const extraProject2 = await createTeamProject('extra-2'); + await shareWorkflowWithProjects(workflow, [ + { project: extraProject1 }, + { project: extraProject2 }, + ]); + + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: true, folderId: folder.id }); + + expect(response.statusCode).toBe(200); + // Folder-scoped — `updatedIds` is omitted from the response. + expect(response.body.data).toEqual({ + updatedCount: 1, + skippedCount: 0, + failedCount: 0, + }); + expect(await readAvailableInMCP(workflow.id)).toBe(true); + }); + + test('disables MCP access for the provided workflows', async () => { + const wf = await createWorkflow( + { name: 'mcp-enabled', settings: { availableInMCP: true } }, + toggleOwner, + ); + + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: false, workflowIds: [wf.id] }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.updatedIds).toEqual([wf.id]); + expect(await readAvailableInMCP(wf.id)).toBe(false); + }); + + test('is idempotent when workflows are already in the requested state', async () => { + const alreadyEnabled = await createWorkflow( + { name: 'already-enabled', settings: { availableInMCP: true } }, + toggleOwner, + ); + const freshlyChanged = await createWorkflow( + { name: 'freshly-changed', settings: {} }, + toggleOwner, + ); + + const response = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ + availableInMCP: true, + workflowIds: [alreadyEnabled.id, freshlyChanged.id], + }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.updatedCount).toBe(2); + expect(response.body.data.updatedIds.sort()).toEqual( + [alreadyEnabled.id, freshlyChanged.id].sort(), + ); + expect(response.body.data.skippedCount).toBe(0); + + // Re-submitting the same request is a no-op but still reports both as updated. + const second = await testServer + .authAgentFor(toggleOwner) + .patch('/mcp/workflows/toggle-access') + .send({ + availableInMCP: true, + workflowIds: [alreadyEnabled.id, freshlyChanged.id], + }); + + expect(second.statusCode).toBe(200); + expect(second.body.data.updatedCount).toBe(2); + expect(second.body.data.skippedCount).toBe(0); + }); + + test('requires authentication', async () => { + const response = await testServer.authlessAgent + .patch('/mcp/workflows/toggle-access') + .send({ availableInMCP: true, workflowIds: ['wf-1'] }); + + expect(response.statusCode).toBe(401); + }); +}); 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 7033551f025..bb4dffa8fd9 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 @@ -1,24 +1,19 @@ import { Logger, ModuleRegistry } from '@n8n/backend-common'; -import { type ApiKey, type AuthenticatedRequest, WorkflowEntity, User, Role } from '@n8n/db'; +import { type ApiKey, type AuthenticatedRequest, User, Role } from '@n8n/db'; import { Container } from '@n8n/di'; import type { Response } from 'express'; import { mock, mockDeep } from 'jest-mock-extended'; -import { HTTP_REQUEST_NODE_TYPE, WEBHOOK_NODE_TYPE, type INode } from 'n8n-workflow'; -import { v4 as uuid } from 'uuid'; import type { ListQuery } from '@/requests'; +import { WorkflowService } from '@/workflows/workflow.service'; import { UpdateMcpSettingsDto } from '../dto/update-mcp-settings.dto'; +import { UpdateWorkflowsAvailabilityDto } from '../dto/update-workflows-availability.dto'; import { McpServerApiKeyService } from '../mcp-api-key.service'; import { McpSettingsController } from '../mcp.settings.controller'; import { McpSettingsService } from '../mcp.settings.service'; import { createWorkflow } from './mock.utils'; -import { CollaborationService } from '@/collaboration/collaboration.service'; -import { NotFoundError } from '@/errors/response-errors/not-found.error'; -import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; -import { WorkflowService } from '@/workflows/workflow.service'; - const createReq = ( body: unknown, overrides: Partial = {}, @@ -83,9 +78,7 @@ describe('McpSettingsController', () => { const moduleRegistry = mockDeep(); const mcpSettingsService = mock(); const mcpServerApiKeyService = mockDeep(); - const workflowFinderService = mock(); const workflowService = mock(); - const collaborationService = mock(); let controller: McpSettingsController; @@ -95,14 +88,7 @@ describe('McpSettingsController', () => { Container.set(McpSettingsService, mcpSettingsService); Container.set(ModuleRegistry, moduleRegistry); Container.set(McpServerApiKeyService, mcpServerApiKeyService); - Container.set(WorkflowFinderService, workflowFinderService); Container.set(WorkflowService, workflowService); - Container.set(CollaborationService, collaborationService); - // Default resolved broadcast — the controller fires this without - // awaiting and attaches a `.catch(...)`, so the mock must return a - // real Promise. Tests that exercise the failure path override this - // with `mockRejectedValueOnce`. - collaborationService.broadcastWorkflowSettingsUpdated.mockResolvedValue(undefined); controller = Container.get(McpSettingsController); }); @@ -315,222 +301,68 @@ describe('McpSettingsController', () => { }); }); - describe('toggleWorkflowMCPAccess', () => { + describe('toggleWorkflowsMCPAccess', () => { const user = createUser(); - const workflowId = 'workflow-1'; - const createWebhookNode = (overrides: Partial = {}): INode => ({ - id: 'node-1', - name: 'Webhook', - type: WEBHOOK_NODE_TYPE, - typeVersion: 1, - position: [0, 0], - parameters: {}, - ...overrides, - }); - - test('throws when workflow cannot be accessed', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue(null); - const req = createReq({}, { user }); - - await expect( - controller.toggleWorkflowMCPAccess(req, mock(), workflowId, { - availableInMCP: true, - }), - ).rejects.toThrow( - new NotFoundError( - 'Could not load the workflow - you can only access workflows available to you', - ), - ); - expect(workflowService.update).not.toHaveBeenCalled(); - }); - - test('allows enabling MCP for inactive workflows', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue( - createWorkflow({ - activeVersionId: null, - nodes: [createWebhookNode({ disabled: false })], - }), - ); - workflowService.update.mockResolvedValue({ - id: workflowId, - settings: { saveManualExecutions: true, availableInMCP: true }, - versionId: 'client-version', - } as unknown as WorkflowEntity); - - await controller.toggleWorkflowMCPAccess( - createReq({}, { user }), - mock(), - workflowId, - { - availableInMCP: true, - }, - ); - - expect(workflowService.update).toHaveBeenCalledTimes(1); - }); - - test('allows disabling MCP for inactive workflows', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue( - createWorkflow({ activeVersionId: null }), - ); - 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('allows enabling MCP regardless of trigger node types', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue( - createWorkflow({ - activeVersionId: uuid(), - nodes: [ - { - id: 'node-2', - name: 'HTTP Request', - type: HTTP_REQUEST_NODE_TYPE, - typeVersion: 1, - position: [10, 10], - parameters: {}, - }, - ], - }), - ); - workflowService.update.mockResolvedValue({ - id: workflowId, - settings: { saveManualExecutions: true, availableInMCP: true }, - versionId: 'client-version', - } as unknown as WorkflowEntity); - - await controller.toggleWorkflowMCPAccess( - createReq({}, { user }), - mock(), - workflowId, - { - availableInMCP: true, - }, - ); - - expect(workflowService.update).toHaveBeenCalledTimes(1); - }); - - test('persists MCP availability when validation passes', async () => { - const workflow = createWorkflow({ - activeVersionId: uuid(), - settings: { saveManualExecutions: true }, - nodes: [ - createWebhookNode({ disabled: false }), - { - id: 'node-2', - name: 'HTTP Request', - type: HTTP_REQUEST_NODE_TYPE, - typeVersion: 1, - position: [10, 10], - parameters: {}, - }, - ], - }); - workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow); - workflowService.update.mockResolvedValue({ - id: workflowId, - settings: { saveManualExecutions: true, availableInMCP: true }, - versionId: 'updated-version-id', - } as unknown as WorkflowEntity); - - const req = createReq({}, { user }); - const response = await controller.toggleWorkflowMCPAccess(req, mock(), workflowId, { + test('delegates to mcpSettingsService.bulkSetAvailableInMCP and returns its result', async () => { + const dto = new UpdateWorkflowsAvailabilityDto({ availableInMCP: true, + workflowIds: ['wf-1', 'wf-2'], }); + const bulkResult = { + updatedCount: 2, + updatedIds: ['wf-1', 'wf-2'], + skippedCount: 0, + failedCount: 0, + }; + mcpSettingsService.bulkSetAvailableInMCP.mockResolvedValue(bulkResult); - expect(workflowService.update).toHaveBeenCalledTimes(1); - const updateArgs = workflowService.update.mock.calls[0]; - expect(updateArgs[0]).toEqual(user); - expect(updateArgs[1]).toBeInstanceOf(WorkflowEntity); - expect(updateArgs[1].settings).toEqual({ saveManualExecutions: true, availableInMCP: true }); - expect(updateArgs[1].versionId).toEqual('some-version-id'); - expect(updateArgs[2]).toEqual(workflowId); + const req = createReq({}, { user }); + const result = await controller.toggleWorkflowsMCPAccess(req, mock(), dto); - expect(response).toEqual({ - id: workflowId, - settings: { saveManualExecutions: true, availableInMCP: true }, - versionId: 'updated-version-id', - }); + expect(mcpSettingsService.bulkSetAvailableInMCP).toHaveBeenCalledTimes(1); + expect(mcpSettingsService.bulkSetAvailableInMCP).toHaveBeenCalledWith(user, dto); + expect(result).toEqual(bulkResult); + }); + }); + + describe('UpdateWorkflowsAvailabilityDto', () => { + test('requires availableInMCP to be a boolean', () => { + expect(() => new UpdateWorkflowsAvailabilityDto({} as never)).toThrow(); + expect( + () => new UpdateWorkflowsAvailabilityDto({ availableInMCP: 'yes' } as never), + ).toThrow(); }); - test('broadcasts a settings update with a post-update checksum to open collaborators', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue( - createWorkflow({ activeVersionId: null }), - ); - workflowService.update.mockResolvedValue({ - id: workflowId, - name: 'wf', - nodes: [], - connections: {}, - settings: { availableInMCP: true }, - versionId: 'updated-version-id', - } as unknown as WorkflowEntity); - - await controller.toggleWorkflowMCPAccess( - createReq({}, { user }), - mock(), - workflowId, - { - availableInMCP: true, - }, - ); - - expect(collaborationService.broadcastWorkflowSettingsUpdated).toHaveBeenCalledTimes(1); - const [broadcastWorkflowId, broadcastSettings, broadcastChecksum] = - collaborationService.broadcastWorkflowSettingsUpdated.mock.calls[0]; - expect(broadcastWorkflowId).toBe(workflowId); - expect(broadcastSettings).toEqual({ availableInMCP: true }); - expect(typeof broadcastChecksum).toBe('string'); - expect(broadcastChecksum).toMatch(/^[a-f0-9]{64}$/); - }); - - test('does not fail the request when the broadcast throws', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue( - createWorkflow({ activeVersionId: null }), - ); - workflowService.update.mockResolvedValue({ - id: workflowId, - settings: { availableInMCP: false }, - versionId: 'updated-version-id', - } as unknown as WorkflowEntity); - collaborationService.broadcastWorkflowSettingsUpdated.mockRejectedValueOnce( - new Error('push down'), - ); - - await expect( - controller.toggleWorkflowMCPAccess(createReq({}, { user }), mock(), workflowId, { - availableInMCP: false, - }), - ).resolves.toEqual({ - id: workflowId, - settings: { availableInMCP: false }, - versionId: 'updated-version-id', + test('accepts a valid workflowIds scope', () => { + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1'], }); + expect(dto.workflowIds).toEqual(['wf-1']); + expect(dto.projectId).toBeUndefined(); + expect(dto.folderId).toBeUndefined(); }); - test('does not broadcast when the workflow cannot be accessed', async () => { - workflowFinderService.findWorkflowForUser.mockResolvedValue(null); + test('rejects an empty workflowIds array', () => { + expect( + () => + new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: [], + }), + ).toThrow(); + }); - await expect( - controller.toggleWorkflowMCPAccess(createReq({}, { user }), mock(), workflowId, { - availableInMCP: true, - }), - ).rejects.toThrow(NotFoundError); - - expect(collaborationService.broadcastWorkflowSettingsUpdated).not.toHaveBeenCalled(); + test('rejects workflowIds arrays over the cap', () => { + const workflowIds = Array.from({ length: 101 }, (_, i) => `wf-${i}`); + expect( + () => + new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds, + }), + ).toThrow(); }); }); }); diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts index dddf750ed6e..6f0e6b11662 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts @@ -1,8 +1,15 @@ -import type { Settings, SettingsRepository } from '@n8n/db'; +import type { Logger } from '@n8n/backend-common'; +import type { GlobalConfig } from '@n8n/config'; +import type { Settings, SettingsRepository, User, WorkflowRepository } from '@n8n/db'; +import { WorkflowEntity } from '@n8n/db'; +import type { EntityManager } from '@n8n/typeorm'; import { mock } from 'jest-mock-extended'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import type { CacheService } from '@/services/cache/cache.service'; +import type { WorkflowFinderService } from '@/workflows/workflow-finder.service'; +import { UpdateWorkflowsAvailabilityDto } from '../dto/update-workflows-availability.dto'; import { McpSettingsService } from '../mcp.settings.service'; describe('McpSettingsService', () => { @@ -11,14 +18,29 @@ describe('McpSettingsService', () => { let upsert: jest.Mock; let settingsRepository: SettingsRepository; const cacheService = mock(); + const workflowRepository = mock(); + const workflowFinderService = mock(); + const logger = mock(); + const globalConfig = { + executions: { timeout: -1 }, + } as unknown as GlobalConfig; beforeEach(() => { jest.clearAllMocks(); findByKey = jest.fn, [string]>(); upsert = jest.fn(); settingsRepository = { findByKey, upsert } as unknown as SettingsRepository; + workflowFinderService.hasProjectScopeForUser.mockResolvedValue(true); + workflowFinderService.findProjectIdForFolder.mockResolvedValue('project-1'); - service = new McpSettingsService(settingsRepository, cacheService); + service = new McpSettingsService( + settingsRepository, + cacheService, + workflowRepository, + workflowFinderService, + globalConfig, + logger, + ); }); describe('getEnabled', () => { @@ -65,4 +87,500 @@ describe('McpSettingsService', () => { ); }); }); + + describe('bulkSetAvailableInMCP', () => { + const user = mock({ id: 'user-1' }); + + // Minimal `find`/`update` stub that behaves like an `EntityManager` + // scoped to WorkflowEntity rows the test sets up. Mirrors the + // production `select: ['id', 'settings']` — nothing more is needed + // since we no longer compute checksums or emit events here. + const createTransactionStubs = (seeded: Array & { id: string }>) => { + const storage = new Map( + seeded.map((w) => [w.id, { ...w, isArchived: w.isArchived ?? false }]), + ); + + const find = jest.fn( + async ( + _entity: unknown, + options: { where: { id: { _value: string[] }; isArchived: boolean } }, + ) => { + const ids = options.where.id._value; + return ids + .map((id) => storage.get(id)) + .filter( + (row): row is Partial & { id: string; isArchived: boolean } => + !!row && row.isArchived === options.where.isArchived, + ) + .map((row) => ({ id: row.id, settings: row.settings })); + }, + ); + + const update = jest.fn( + async (_entity: unknown, where: { id: string }, patch: Partial) => { + const existing = storage.get(where.id); + if (!existing) return; + storage.set(where.id, { ...existing, ...patch }); + }, + ); + + const trx = { find, update } as unknown as EntityManager; + const manager = { + transaction: jest.fn(async (run: (trx: EntityManager) => Promise) => { + return await run(trx); + }), + }; + + return { trx, manager, find, update, storage }; + }; + + const setupRepository = (seeded: Array & { id: string }>) => { + const stubs = createTransactionStubs(seeded); + Object.assign(workflowRepository, { manager: stubs.manager }); + return stubs; + }; + + test('throws BadRequestError when no scope is provided', async () => { + const dto = new UpdateWorkflowsAvailabilityDto({ availableInMCP: true }); + + await expect(service.bulkSetAvailableInMCP(user, dto)).rejects.toThrow(BadRequestError); + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).not.toHaveBeenCalled(); + expect(workflowFinderService.findAllWorkflowIdsForUser).not.toHaveBeenCalled(); + }); + + test('throws BadRequestError when multiple scopes are provided', async () => { + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1'], + projectId: 'project-1', + }); + + await expect(service.bulkSetAvailableInMCP(user, dto)).rejects.toThrow(BadRequestError); + }); + + test('filters unauthorized ids and applies updates inside a transaction', async () => { + const stubs = setupRepository([ + { id: 'wf-1', settings: { saveManualExecutions: true } }, + { id: 'wf-2', settings: { availableInMCP: false } }, + ]); + + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue( + new Set(['wf-1', 'wf-2']), + ); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1', 'wf-2', 'wf-unauthorized'], + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).toHaveBeenCalledWith( + ['wf-1', 'wf-2', 'wf-unauthorized'], + user, + ['workflow:update'], + ); + expect(stubs.manager.transaction).toHaveBeenCalledTimes(1); + expect(stubs.update).toHaveBeenCalledTimes(2); + expect(result).toEqual({ + updatedCount: 2, + updatedIds: ['wf-1', 'wf-2'], + // wf-unauthorized was in the request but filtered out — counts as skipped. + skippedCount: 1, + failedCount: 0, + }); + }); + + test('skips archived workflows and reports them as skipped', async () => { + const stubs = setupRepository([ + { id: 'wf-1', settings: {}, isArchived: false }, + { id: 'wf-2', settings: {}, isArchived: true }, + ]); + + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue( + new Set(['wf-1', 'wf-2']), + ); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1', 'wf-2'], + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(stubs.update).toHaveBeenCalledTimes(1); + expect(stubs.update).toHaveBeenCalledWith( + WorkflowEntity, + { id: 'wf-1' }, + expect.objectContaining({ settings: expect.objectContaining({ availableInMCP: true }) }), + ); + expect(result).toEqual({ + updatedCount: 1, + updatedIds: ['wf-1'], + skippedCount: 1, + failedCount: 0, + }); + }); + + test('treats no-op updates as already-in-state for idempotency', async () => { + const stubs = setupRepository([ + // Will be written (value differs). + { id: 'wf-1', settings: { availableInMCP: false } }, + // No-op — already in the requested state. + { id: 'wf-2', settings: { availableInMCP: true } }, + ]); + + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue( + new Set(['wf-1', 'wf-2']), + ); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1', 'wf-2'], + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + // Only wf-1 needed a real write. wf-2 was already in the target state. + expect(stubs.update).toHaveBeenCalledTimes(1); + // Both ids are reported as updated (the DB is in the requested state + // for both). No-ops go last in the list. + expect(result).toEqual({ + updatedCount: 2, + updatedIds: ['wf-1', 'wf-2'], + skippedCount: 0, + failedCount: 0, + }); + }); + + test('idempotent when all workflows are already in requested state', async () => { + const stubs = setupRepository([ + { id: 'wf-1', settings: { availableInMCP: true } }, + { id: 'wf-2', settings: { availableInMCP: true } }, + ]); + + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue( + new Set(['wf-1', 'wf-2']), + ); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1', 'wf-2'], + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(stubs.update).not.toHaveBeenCalled(); + expect(result).toEqual({ + updatedCount: 2, + updatedIds: ['wf-1', 'wf-2'], + skippedCount: 0, + failedCount: 0, + }); + }); + + test('resolves candidates via findAllWorkflowIdsForUser when scoped by projectId', async () => { + const stubs = setupRepository([{ id: 'wf-1', settings: {} }]); + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(['wf-1']); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'project-1', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).not.toHaveBeenCalled(); + expect(workflowFinderService.hasProjectScopeForUser).toHaveBeenCalledWith( + user, + ['workflow:update'], + 'project-1', + ); + expect(workflowFinderService.findAllWorkflowIdsForUser).toHaveBeenCalledWith( + user, + ['workflow:update'], + undefined, + 'project-1', + ); + expect(stubs.update).toHaveBeenCalledTimes(1); + expect(result.updatedCount).toBe(1); + }); + + test('does not resolve project-scoped workflows when user lacks project scope', async () => { + const stubs = setupRepository([]); + workflowFinderService.hasProjectScopeForUser.mockResolvedValueOnce(false); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'project-1', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(workflowFinderService.findAllWorkflowIdsForUser).not.toHaveBeenCalled(); + expect(stubs.manager.transaction).not.toHaveBeenCalled(); + expect(result).toEqual({ + updatedCount: 0, + skippedCount: 0, + failedCount: 0, + }); + }); + + test('omits updatedIds from the response when scoped by projectId', async () => { + setupRepository([ + { id: 'wf-1', settings: {} }, + { id: 'wf-2', settings: {} }, + ]); + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(['wf-1', 'wf-2']); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'project-1', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(result).toEqual({ updatedCount: 2, skippedCount: 0, failedCount: 0 }); + expect(result).not.toHaveProperty('updatedIds'); + }); + + test('omits updatedIds from the response when scoped by folderId', async () => { + setupRepository([{ id: 'wf-1', settings: {} }]); + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(['wf-1']); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + folderId: 'folder-1', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(result).toEqual({ updatedCount: 1, skippedCount: 0, failedCount: 0 }); + expect(result).not.toHaveProperty('updatedIds'); + }); + + test('resolves candidates via findAllWorkflowIdsForUser when scoped by folderId', async () => { + setupRepository([{ id: 'wf-1', settings: {} }]); + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(['wf-1']); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + folderId: 'folder-1', + }); + + await service.bulkSetAvailableInMCP(user, dto); + + expect(workflowFinderService.findProjectIdForFolder).toHaveBeenCalledWith('folder-1'); + expect(workflowFinderService.hasProjectScopeForUser).toHaveBeenCalledWith( + user, + ['workflow:update'], + 'project-1', + ); + expect(workflowFinderService.findAllWorkflowIdsForUser).toHaveBeenCalledWith( + user, + ['workflow:update'], + 'folder-1', + 'project-1', + ); + }); + + test('does not resolve folder-scoped workflows when folder project cannot be scoped', async () => { + const stubs = setupRepository([]); + workflowFinderService.findProjectIdForFolder.mockResolvedValueOnce(null); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + folderId: 'folder-1', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(workflowFinderService.hasProjectScopeForUser).not.toHaveBeenCalled(); + expect(workflowFinderService.findAllWorkflowIdsForUser).not.toHaveBeenCalled(); + expect(stubs.manager.transaction).not.toHaveBeenCalled(); + expect(result).toEqual({ + updatedCount: 0, + skippedCount: 0, + failedCount: 0, + }); + }); + + test('returns zeroed result and does not open a transaction when no candidates are found', async () => { + const stubs = setupRepository([]); + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue([]); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'empty-project', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(stubs.manager.transaction).not.toHaveBeenCalled(); + expect(result).toEqual({ + updatedCount: 0, + skippedCount: 0, + failedCount: 0, + }); + expect(result).not.toHaveProperty('updatedIds'); + }); + + test('returns an empty updatedIds array when scoped by workflowIds and none are accessible', async () => { + setupRepository([]); + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(new Set()); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1', 'wf-2'], + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(result).toEqual({ + updatedCount: 0, + skippedCount: 2, + failedCount: 0, + updatedIds: [], + }); + }); + + test('chunks large candidate sets into one transaction per chunk', async () => { + // 600 workflows -> chunked into 500 + 100 with BULK_CHUNK_SIZE = 500. + const seeded = Array.from({ length: 600 }, (_, i) => ({ + id: `wf-${i}`, + settings: {} as Record, + })); + const stubs = setupRepository(seeded); + + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(seeded.map((w) => w.id)); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'project-big', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + // Two transactions (one per chunk), each with its own find call. + expect(stubs.manager.transaction).toHaveBeenCalledTimes(2); + expect(stubs.find).toHaveBeenCalledTimes(2); + expect(result.updatedCount).toBe(600); + expect(result.failedCount).toBe(0); + }); + + test('continues processing when a chunk transaction fails, and reports failedCount', async () => { + // 600 workflows -> 2 chunks + const seeded = Array.from({ length: 600 }, (_, i) => ({ + id: `wf-${i}`, + settings: {} as Record, + })); + const stubs = setupRepository(seeded); + + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(seeded.map((w) => w.id)); + + // Force the first chunk's transaction to fail; the second runs normally. + const originalTransaction = stubs.manager.transaction; + stubs.manager.transaction + .mockImplementationOnce(async () => { + throw new Error('chunk transaction failed'); + }) + .mockImplementationOnce(originalTransaction.getMockImplementation()!); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'project-big', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(stubs.manager.transaction).toHaveBeenCalledTimes(2); + // Second chunk (100 workflows) committed successfully. + expect(result.updatedCount).toBe(100); + // First chunk (500 workflows) is counted as failed — the + // transaction rolled back so no rows were written. + expect(result.failedCount).toBe(500); + expect(result.skippedCount).toBe(0); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('Failed to bulk-update workflow MCP availability'), + expect.objectContaining({ + error: expect.any(Error), + chunkSize: 500, + chunkStart: 0, + availableInMCP: true, + }), + ); + }); + + test('surfaces chunk failures in updatedIds when scoped by workflowIds', async () => { + const stubs = setupRepository([{ id: 'wf-1', settings: {} }]); + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(new Set(['wf-1'])); + + stubs.manager.transaction.mockImplementationOnce(async () => { + throw new Error('chunk transaction failed'); + }); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1'], + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(result).toEqual({ + updatedCount: 0, + skippedCount: 0, + failedCount: 1, + updatedIds: [], + }); + }); + + test('commits successful chunks independently when a later chunk fails', async () => { + const seeded = Array.from({ length: 600 }, (_, i) => ({ + id: `wf-${i}`, + settings: { availableInMCP: false } as Record, + })); + const stubs = setupRepository(seeded); + + workflowFinderService.findAllWorkflowIdsForUser.mockResolvedValue(seeded.map((w) => w.id)); + + const originalTransaction = stubs.manager.transaction.getMockImplementation()!; + stubs.manager.transaction + .mockImplementationOnce(originalTransaction) + .mockImplementationOnce(async () => { + throw new Error('second chunk failed'); + }); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + projectId: 'project-big', + }); + + const result = await service.bulkSetAvailableInMCP(user, dto); + + expect(result.updatedCount).toBe(500); + expect(result.failedCount).toBe(100); + expect(stubs.storage.get('wf-0')?.settings?.availableInMCP).toBe(true); + expect(stubs.storage.get('wf-499')?.settings?.availableInMCP).toBe(true); + expect(stubs.storage.get('wf-500')?.settings?.availableInMCP).toBe(false); + expect(stubs.storage.get('wf-599')?.settings?.availableInMCP).toBe(false); + }); + + test('deduplicates workflowIds before looking up access', async () => { + setupRepository([{ id: 'wf-1', settings: {} }]); + workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(new Set(['wf-1'])); + + const dto = new UpdateWorkflowsAvailabilityDto({ + availableInMCP: true, + workflowIds: ['wf-1', 'wf-1', 'wf-1'], + }); + + await service.bulkSetAvailableInMCP(user, dto); + + expect(workflowFinderService.findWorkflowIdsWithScopeForUser).toHaveBeenCalledWith( + ['wf-1'], + user, + ['workflow:update'], + ); + }); + }); }); diff --git a/packages/cli/src/modules/mcp/dto/update-workflow-availability.dto.ts b/packages/cli/src/modules/mcp/dto/update-workflow-availability.dto.ts deleted file mode 100644 index 76decd68a30..00000000000 --- a/packages/cli/src/modules/mcp/dto/update-workflow-availability.dto.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { Z } from '@n8n/api-types'; -import { z } from 'zod'; - -export class UpdateWorkflowAvailabilityDto extends Z.class({ - availableInMCP: z.boolean(), -}) {} diff --git a/packages/cli/src/modules/mcp/dto/update-workflows-availability.dto.ts b/packages/cli/src/modules/mcp/dto/update-workflows-availability.dto.ts new file mode 100644 index 00000000000..f21440cfef8 --- /dev/null +++ b/packages/cli/src/modules/mcp/dto/update-workflows-availability.dto.ts @@ -0,0 +1,9 @@ +import { Z } from '@n8n/api-types'; +import { z } from 'zod'; + +export class UpdateWorkflowsAvailabilityDto extends Z.class({ + availableInMCP: z.boolean(), + workflowIds: z.array(z.string().min(1)).min(1).max(100).optional(), + projectId: z.string().min(1).optional(), + folderId: z.string().min(1).optional(), +}) {} diff --git a/packages/cli/src/modules/mcp/mcp.settings.controller.ts b/packages/cli/src/modules/mcp/mcp.settings.controller.ts index 6fed0e5e8a5..0a1a5ff8477 100644 --- a/packages/cli/src/modules/mcp/mcp.settings.controller.ts +++ b/packages/cli/src/modules/mcp/mcp.settings.controller.ts @@ -1,30 +1,17 @@ import { ModuleRegistry, Logger } from '@n8n/backend-common'; -import { type AuthenticatedRequest, WorkflowEntity } from '@n8n/db'; -import { - Body, - Post, - Get, - Patch, - RestController, - GlobalScope, - Param, - ProjectScope, -} from '@n8n/decorators'; +import { type AuthenticatedRequest } from '@n8n/db'; +import { Body, Post, Get, Patch, RestController, GlobalScope } from '@n8n/decorators'; import type { Response } from 'express'; -import { calculateWorkflowChecksum } from 'n8n-workflow'; -import { UpdateMcpSettingsDto } from './dto/update-mcp-settings.dto'; -import { UpdateWorkflowAvailabilityDto } from './dto/update-workflow-availability.dto'; -import { McpServerApiKeyService } from './mcp-api-key.service'; -import { McpSettingsService } from './mcp.settings.service'; - -import { CollaborationService } from '@/collaboration/collaboration.service'; -import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { listQueryMiddleware } from '@/middlewares'; import type { ListQuery } from '@/requests'; -import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; import { WorkflowService } from '@/workflows/workflow.service'; +import { UpdateMcpSettingsDto } from './dto/update-mcp-settings.dto'; +import { UpdateWorkflowsAvailabilityDto } from './dto/update-workflows-availability.dto'; +import { McpServerApiKeyService } from './mcp-api-key.service'; +import { McpSettingsService } from './mcp.settings.service'; + @RestController('/mcp') export class McpSettingsController { constructor( @@ -32,9 +19,7 @@ export class McpSettingsController { private readonly logger: Logger, private readonly moduleRegistry: ModuleRegistry, private readonly mcpServerApiKeyService: McpServerApiKeyService, - private readonly workflowFinderService: WorkflowFinderService, private readonly workflowService: WorkflowService, - private readonly collaborationService: CollaborationService, ) {} @GlobalScope('mcp:manage') @@ -91,60 +76,13 @@ export class McpSettingsController { res.json({ count, data: workflows }); } - @ProjectScope('workflow:update') - @Patch('/workflows/:workflowId/toggle-access') - async toggleWorkflowMCPAccess( + // Ideally we would use ProjectScope here but it only works if projectId is a URL parameter + @Patch('/workflows/toggle-access') + async toggleWorkflowsMCPAccess( req: AuthenticatedRequest, _res: Response, - @Param('workflowId') workflowId: string, - @Body dto: UpdateWorkflowAvailabilityDto, + @Body dto: UpdateWorkflowsAvailabilityDto, ) { - const workflow = await this.workflowFinderService.findWorkflowForUser( - workflowId, - req.user, - ['workflow:update'], - { includeActiveVersion: true }, - ); - - if (!workflow) { - this.logger.warn('User attempted to update MCP availability without permissions', { - workflowId, - userId: req.user.id, - }); - throw new NotFoundError( - 'Could not load the workflow - you can only access workflows available to you', - ); - } - - const workflowUpdate = new WorkflowEntity(); - const currentSettings = workflow.settings ?? {}; - workflowUpdate.settings = { - ...currentSettings, - availableInMCP: dto.availableInMCP, - }; - workflowUpdate.versionId = workflow.versionId; - - const updatedWorkflow = await this.workflowService.update(req.user, workflowUpdate, workflowId); - - const checksum = await calculateWorkflowChecksum(updatedWorkflow); - - void this.collaborationService - .broadcastWorkflowSettingsUpdated( - workflowId, - { availableInMCP: dto.availableInMCP }, - checksum, - ) - .catch((error) => { - this.logger.warn('Failed to broadcast workflow settings update', { - workflowId, - cause: error instanceof Error ? error.message : String(error), - }); - }); - - return { - id: updatedWorkflow.id, - settings: updatedWorkflow.settings, - versionId: updatedWorkflow.versionId, - }; + return await this.mcpSettingsService.bulkSetAvailableInMCP(req.user, dto); } } diff --git a/packages/cli/src/modules/mcp/mcp.settings.service.ts b/packages/cli/src/modules/mcp/mcp.settings.service.ts index 814ec702e95..982dd53f434 100644 --- a/packages/cli/src/modules/mcp/mcp.settings.service.ts +++ b/packages/cli/src/modules/mcp/mcp.settings.service.ts @@ -1,15 +1,37 @@ -import { SettingsRepository } from '@n8n/db'; +import { Logger } from '@n8n/backend-common'; +import { GlobalConfig } from '@n8n/config'; +import type { User } from '@n8n/db'; +import { SettingsRepository, WorkflowEntity, WorkflowRepository } from '@n8n/db'; import { Service } from '@n8n/di'; +import { In } from '@n8n/typeorm'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { CacheService } from '@/services/cache/cache.service'; +import { removeDefaultValues } from '@/workflow-helpers'; +import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; + +import type { UpdateWorkflowsAvailabilityDto } from './dto/update-workflows-availability.dto'; const KEY = 'mcp.access.enabled'; +const BULK_CHUNK_SIZE = 500; + +type BulkSetAvailableInMCPResult = { + updatedCount: number; + skippedCount: number; + failedCount: number; + updatedIds?: string[]; +}; + @Service() export class McpSettingsService { constructor( private readonly settingsRepository: SettingsRepository, private readonly cacheService: CacheService, + private readonly workflowRepository: WorkflowRepository, + private readonly workflowFinderService: WorkflowFinderService, + private readonly globalConfig: GlobalConfig, + private readonly logger: Logger, ) {} async getEnabled(): Promise { @@ -36,4 +58,140 @@ export class McpSettingsService { await this.cacheService.set(KEY, enabled.toString()); } + + async bulkSetAvailableInMCP( + user: User, + dto: UpdateWorkflowsAvailabilityDto, + ): Promise { + const { availableInMCP, workflowIds, projectId, folderId } = dto; + + const scopeCount = [workflowIds, projectId, folderId].filter(Boolean).length; + if (scopeCount !== 1) { + throw new BadRequestError('Provide exactly one of workflowIds, projectId or folderId'); + } + + const candidateIds = await this.resolveCandidateIds(user, { + workflowIds, + projectId, + folderId, + }); + + const isWorkflowIdsScope = Boolean(workflowIds); + const baselineSize = isWorkflowIdsScope ? new Set(workflowIds).size : candidateIds.length; + + if (candidateIds.length === 0) { + return { + updatedCount: 0, + skippedCount: baselineSize, + failedCount: 0, + ...(isWorkflowIdsScope ? { updatedIds: [] } : {}), + }; + } + + const writtenIds: string[] = []; + const noOpIds: string[] = []; + let failedCount = 0; + + for (let start = 0; start < candidateIds.length; start += BULK_CHUNK_SIZE) { + const chunk = candidateIds.slice(start, start + BULK_CHUNK_SIZE); + + try { + const chunkResult = await this.workflowRepository.manager.transaction(async (trx) => { + const chunkWritten: string[] = []; + const chunkNoOp: string[] = []; + const now = new Date(); + + const rows = await trx.find(WorkflowEntity, { + where: { id: In(chunk), isArchived: false }, + select: ['id', 'settings'], + }); + + for (const row of rows) { + if (row.settings?.availableInMCP === availableInMCP) { + chunkNoOp.push(row.id); + continue; + } + + const nextSettings = removeDefaultValues( + { ...(row.settings ?? {}), availableInMCP }, + this.globalConfig.executions.timeout, + ); + + await trx.update( + WorkflowEntity, + { id: row.id }, + { settings: nextSettings, updatedAt: now }, + ); + + chunkWritten.push(row.id); + } + + return { written: chunkWritten, noOp: chunkNoOp }; + }); + + writtenIds.push(...chunkResult.written); + noOpIds.push(...chunkResult.noOp); + } catch (error) { + failedCount += chunk.length; + this.logger.error('Failed to bulk-update workflow MCP availability for chunk', { + error, + chunkSize: chunk.length, + chunkStart: start, + availableInMCP, + }); + } + } + + const confirmedIds = [...writtenIds, ...noOpIds]; + + return { + updatedCount: confirmedIds.length, + skippedCount: Math.max(0, baselineSize - confirmedIds.length - failedCount), + failedCount, + ...(isWorkflowIdsScope ? { updatedIds: confirmedIds } : {}), + }; + } + + private async resolveCandidateIds( + user: User, + scope: { + workflowIds?: string[]; + projectId?: string; + folderId?: string; + }, + ): Promise { + if (scope.workflowIds) { + const uniqueIds = [...new Set(scope.workflowIds)]; + const accessibleIds = await this.workflowFinderService.findWorkflowIdsWithScopeForUser( + uniqueIds, + user, + ['workflow:update'], + ); + return uniqueIds.filter((id) => accessibleIds.has(id)); + } + + const projectId = + scope.projectId ?? + (scope.folderId + ? await this.workflowFinderService.findProjectIdForFolder(scope.folderId) + : null); + + if ( + projectId === null || + !(await this.workflowFinderService.hasProjectScopeForUser( + user, + ['workflow:update'], + projectId, + )) + ) { + return []; + } + + return await this.workflowFinderService.findAllWorkflowIdsForUser( + user, + ['workflow:update'], + scope.folderId, + projectId, + ); + } } diff --git a/packages/cli/src/workflows/workflow-finder.service.ts b/packages/cli/src/workflows/workflow-finder.service.ts index 8e597d4afb0..5c231ec9ae6 100644 --- a/packages/cli/src/workflows/workflow-finder.service.ts +++ b/packages/cli/src/workflows/workflow-finder.service.ts @@ -7,6 +7,7 @@ import type { EntityManager, FindOptionsWhere } from '@n8n/typeorm'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; +import { userHasScopes } from '@/permissions.ee/check-access'; import { RoleService } from '@/services/role.service'; @Service() @@ -122,6 +123,19 @@ export class WorkflowFinderService { return new Set(sharedWorkflows.map((sw) => sw.workflowId)); } + async hasProjectScopeForUser(user: User, scopes: Scope[], projectId: string) { + return await userHasScopes(user, scopes, false, { projectId }); + } + + async findProjectIdForFolder(folderId: string): Promise { + const folder = await this.folderRepository.findOne({ + where: { id: folderId }, + relations: { homeProject: true }, + }); + + return folder?.homeProject.id ?? null; + } + async findAllWorkflowIdsForUser( user: User, scopes: Scope[], @@ -134,7 +148,7 @@ export class WorkflowFinderService { where, }); - return sharedWorkflows.map(({ workflowId }) => workflowId); + return Array.from(new Set(sharedWorkflows.map(({ workflowId }) => workflowId))); } async findAllWorkflowsForUser( diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 4bc2af30d03..986fe3b58d3 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3777,6 +3777,7 @@ "workflowSettings.availableInMCP.tooltip": "Make this workflow visible to AI Agents through n8n MCP", "workflowSettings.toggleMCP.error.title": "Error updating MCP settings", "workflowSettings.toggleMCP.notFoundError": "Workflow not found", + "workflowSettings.toggleMCP.updateSkippedError": "Workflow {workflowId} could not be updated. It may be archived or you may no longer have permission to edit it.", "workflowHistory.title": "Version History", "workflowHistory.content.actions": "Actions", "workflowHistory.item.id": "ID: {id}", diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.api.ts b/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.api.ts index 2d34e764aa9..42c245c3e80 100644 --- a/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.api.ts +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.api.ts @@ -3,7 +3,7 @@ import type { ListOAuthClientsResponseDto, DeleteOAuthClientResponseDto, } from '@n8n/api-types'; -import type { IWorkflowSettings, WorkflowListItem } from '@/Interface'; +import type { WorkflowListItem } from '@/Interface'; import type { IRestApiContext } from '@n8n/rest-api-client'; import { makeRestApiRequest, getFullApiResponse } from '@n8n/rest-api-client'; @@ -11,6 +11,18 @@ export type McpSettingsResponse = { mcpAccessEnabled: boolean; }; +export type ToggleWorkflowsMcpAccessTarget = + | { workflowIds: string[] } + | { projectId: string } + | { folderId: string }; + +export type ToggleWorkflowsMcpAccessResponse = { + updatedCount: number; + skippedCount: number; + failedCount: number; + updatedIds?: string[]; +}; + export async function getMcpSettings(context: IRestApiContext): Promise { return await makeRestApiRequest(context, 'GET', '/mcp/settings'); } @@ -32,19 +44,19 @@ export async function rotateApiKey(context: IRestApiContext): Promise { return await makeRestApiRequest(context, 'POST', '/mcp/api-key/rotate'); } -export async function toggleWorkflowMcpAccessApi( +/** + * Bulk-toggles MCP availability for a set of workflows scoped by either an + * explicit id list, a project, or a folder (+ its descendants). + */ +export async function toggleWorkflowsMcpAccessApi( context: IRestApiContext, - workflowId: string, + target: ToggleWorkflowsMcpAccessTarget, availableInMCP: boolean, -): Promise<{ id: string; settings: IWorkflowSettings | undefined; versionId: string }> { - return await makeRestApiRequest( - context, - 'PATCH', - `/mcp/workflows/${encodeURIComponent(workflowId)}/toggle-access`, - { - availableInMCP, - }, - ); +): Promise { + return await makeRestApiRequest(context, 'PATCH', '/mcp/workflows/toggle-access', { + availableInMCP, + ...target, + }); } export async function fetchOAuthClients( diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.test.ts b/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.test.ts new file mode 100644 index 00000000000..484a9632f4d --- /dev/null +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.test.ts @@ -0,0 +1,257 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { setActivePinia, createPinia } from 'pinia'; + +import * as mcpApi from './mcp.api'; +import { useMCPStore } from './mcp.store'; +import { useWorkflowsStore } from '@/app/stores/workflows.store'; +import { useWorkflowsListStore } from '@/app/stores/workflowsList.store'; + +const { mockWorkflowDocumentStore } = vi.hoisted(() => ({ + mockWorkflowDocumentStore: { + allNodes: [], + name: '', + settings: {}, + mergeSettings: vi.fn(), + getPinDataSnapshot: vi.fn().mockReturnValue({}), + getNodeByName: vi.fn().mockReturnValue(null), + }, +})); + +vi.mock('@n8n/stores/useRootStore', () => ({ + useRootStore: () => ({ restApiContext: {} }), +})); + +vi.mock('@/app/stores/workflowDocument.store', () => ({ + useWorkflowDocumentStore: vi.fn(() => mockWorkflowDocumentStore), + createWorkflowDocumentId: (id: string) => id, +})); + +describe('mcp.store', () => { + let store: ReturnType; + let workflowsStore: ReturnType; + let workflowsListStore: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + setActivePinia(createPinia()); + store = useMCPStore(); + workflowsStore = useWorkflowsStore(); + workflowsListStore = useWorkflowsListStore(); + }); + + describe('toggleWorkflowMcpAccess', () => { + it('patches the list store entry when the backend confirms the update', async () => { + workflowsListStore.workflowsById = { + 'wf-1': { + id: 'wf-1', + name: 'wf', + settings: { availableInMCP: false, executionOrder: 'v1' }, + }, + } as unknown as typeof workflowsListStore.workflowsById; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 1, + updatedIds: ['wf-1'], + skippedCount: 0, + failedCount: 0, + }); + + await store.toggleWorkflowMcpAccess('wf-1', true); + + expect(workflowsListStore.workflowsById['wf-1'].settings?.availableInMCP).toBe(true); + }); + + it('creates a settings object on list entries that have none', async () => { + workflowsListStore.workflowsById = { + 'wf-1': { + id: 'wf-1', + name: 'wf', + // No `settings` object on this entry — simulates legacy + // workflows or sparse list responses. + }, + } as unknown as typeof workflowsListStore.workflowsById; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 1, + updatedIds: ['wf-1'], + skippedCount: 0, + failedCount: 0, + }); + + await store.toggleWorkflowMcpAccess('wf-1', true); + + expect(workflowsListStore.workflowsById['wf-1'].settings?.availableInMCP).toBe(true); + }); + + it('merges settings into the active workflow document when toggling its own id', async () => { + workflowsStore.workflow.id = 'wf-current'; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 1, + updatedIds: ['wf-current'], + skippedCount: 0, + failedCount: 0, + }); + + await store.toggleWorkflowMcpAccess('wf-current', true); + + expect(mockWorkflowDocumentStore.mergeSettings).toHaveBeenCalledWith({ + availableInMCP: true, + }); + }); + + it('throws when the backend silently skipped the workflow', async () => { + workflowsListStore.workflowsById = { + 'wf-1': { + id: 'wf-1', + name: 'wf', + settings: { availableInMCP: false, executionOrder: 'v1' }, + }, + } as unknown as typeof workflowsListStore.workflowsById; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 0, + updatedIds: [], + skippedCount: 1, + failedCount: 0, + }); + + await expect(store.toggleWorkflowMcpAccess('wf-1', true)).rejects.toThrow( + /could not be updated/i, + ); + + // Local store must remain untouched when the backend rejected the change. + expect(workflowsListStore.workflowsById['wf-1'].settings?.availableInMCP).toBe(false); + expect(mockWorkflowDocumentStore.mergeSettings).not.toHaveBeenCalled(); + }); + + it('propagates network errors without patching local state', async () => { + workflowsListStore.workflowsById = { + 'wf-1': { + id: 'wf-1', + name: 'wf', + settings: { availableInMCP: false, executionOrder: 'v1' }, + }, + } as unknown as typeof workflowsListStore.workflowsById; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockRejectedValue(new Error('network')); + + await expect(store.toggleWorkflowMcpAccess('wf-1', true)).rejects.toThrow('network'); + + expect(workflowsListStore.workflowsById['wf-1'].settings?.availableInMCP).toBe(false); + expect(mockWorkflowDocumentStore.mergeSettings).not.toHaveBeenCalled(); + }); + }); + + describe('toggleWorkflowsMcpAccess (bulk)', () => { + it('applies the new value only to workflows the backend confirmed were updated', async () => { + workflowsListStore.workflowsById = { + 'wf-1': { + id: 'wf-1', + name: 'wf-1', + settings: { availableInMCP: false, executionOrder: 'v1' }, + }, + 'wf-2': { + id: 'wf-2', + name: 'wf-2', + settings: { availableInMCP: false, executionOrder: 'v1' }, + }, + } as unknown as typeof workflowsListStore.workflowsById; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 1, + updatedIds: ['wf-1'], + skippedCount: 1, + failedCount: 0, + }); + + const response = await store.toggleWorkflowsMcpAccess( + { workflowIds: ['wf-1', 'wf-2'] }, + true, + ); + + expect(response.updatedIds).toEqual(['wf-1']); + expect(workflowsListStore.workflowsById['wf-1'].settings?.availableInMCP).toBe(true); + // Skipped workflow remains untouched. + expect(workflowsListStore.workflowsById['wf-2'].settings?.availableInMCP).toBe(false); + }); + + it('does not throw when none of the targeted workflows were updated', async () => { + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 0, + updatedIds: [], + skippedCount: 2, + failedCount: 0, + }); + + await expect( + store.toggleWorkflowsMcpAccess({ workflowIds: ['wf-1', 'wf-2'] }, true), + ).resolves.toEqual({ + updatedCount: 0, + updatedIds: [], + skippedCount: 2, + failedCount: 0, + }); + }); + + it('forwards projectId scope to the API', async () => { + // Project-scoped responses from the backend omit `updatedIds`. + const spy = vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 0, + skippedCount: 0, + failedCount: 0, + }); + + await store.toggleWorkflowsMcpAccess({ projectId: 'project-1' }, false); + + expect(spy).toHaveBeenCalledWith({}, { projectId: 'project-1' }, false); + }); + + it('forwards folderId scope to the API', async () => { + // Folder-scoped responses from the backend omit `updatedIds`. + const spy = vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 0, + skippedCount: 0, + failedCount: 0, + }); + + await store.toggleWorkflowsMcpAccess({ folderId: 'folder-1' }, true); + + expect(spy).toHaveBeenCalledWith({}, { folderId: 'folder-1' }, true); + }); + + it('does not patch local stores when the response omits updatedIds (scope mode)', async () => { + workflowsListStore.workflowsById = { + 'wf-1': { + id: 'wf-1', + name: 'wf-1', + settings: { availableInMCP: false, executionOrder: 'v1' }, + }, + } as unknown as typeof workflowsListStore.workflowsById; + + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 5, + skippedCount: 0, + failedCount: 0, + }); + + await expect( + store.toggleWorkflowsMcpAccess({ projectId: 'project-1' }, true), + ).resolves.toEqual({ updatedCount: 5, skippedCount: 0, failedCount: 0 }); + + expect(workflowsListStore.workflowsById['wf-1'].settings?.availableInMCP).toBe(false); + }); + + it('surfaces partial failures from the backend via failedCount', async () => { + vi.spyOn(mcpApi, 'toggleWorkflowsMcpAccessApi').mockResolvedValue({ + updatedCount: 500, + skippedCount: 0, + failedCount: 100, + }); + + await expect( + store.toggleWorkflowsMcpAccess({ projectId: 'big-project' }, true), + ).resolves.toEqual({ updatedCount: 500, skippedCount: 0, failedCount: 100 }); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.ts b/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.ts index 192b9b029cf..82c4bbe5328 100644 --- a/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/mcp.store.ts @@ -6,21 +6,24 @@ import { useWorkflowDocumentStore, createWorkflowDocumentId, } from '@/app/stores/workflowDocument.store'; -import type { WorkflowListItem } from '@/Interface'; +import type { IWorkflowSettings, WorkflowListItem } from '@/Interface'; import { useRootStore } from '@n8n/stores/useRootStore'; import { updateMcpSettings, - toggleWorkflowMcpAccessApi, + toggleWorkflowsMcpAccessApi, fetchApiKey, rotateApiKey, fetchOAuthClients, deleteOAuthClient, fetchMcpEligibleWorkflows, + type ToggleWorkflowsMcpAccessResponse, + type ToggleWorkflowsMcpAccessTarget, } from '@/features/ai/mcpAccess/mcp.api'; import { computed, ref } from 'vue'; import { useSettingsStore } from '@/app/stores/settings.store'; import { isWorkflowListItem } from '@/app/utils/typeGuards'; import type { ApiKey, OAuthClientResponseDto, DeleteOAuthClientResponseDto } from '@n8n/api-types'; +import { i18n } from '@n8n/i18n'; export const useMCPStore = defineStore(MCP_STORE, () => { const workflowsStore = useWorkflowsStore(); @@ -62,40 +65,62 @@ export const useMCPStore = defineStore(MCP_STORE, () => { return updated; } + function applyAvailableInMCPToLocalStores(workflowId: string, availableInMCP: boolean) { + const existing = workflowsListStore.workflowsById[workflowId]; + if (existing) { + if (existing.settings) { + existing.settings.availableInMCP = availableInMCP; + } else { + existing.settings = { availableInMCP } as IWorkflowSettings; + } + } + + if (workflowId === workflowsStore.workflowId) { + const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId(workflowId)); + workflowDocumentStore.mergeSettings({ availableInMCP }); + } + } + + // Toggle MCP access for a single workflow async function toggleWorkflowMcpAccess( workflowId: string, availableInMCP: boolean, - ): Promise<{ - id: string; - settings: { availableInMCP?: boolean } | undefined; - versionId: string; - }> { - const response = await toggleWorkflowMcpAccessApi( + ): Promise { + const response = await toggleWorkflowsMcpAccessApi( rootStore.restApiContext, - workflowId, + { workflowIds: [workflowId] }, availableInMCP, ); - const { id, settings, versionId } = response; - - // Update local version of the workflow - if (id === workflowsStore.workflowId) { - const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId(id)); - workflowDocumentStore.setVersionData({ - versionId, - name: workflowDocumentStore.versionData?.name ?? null, - description: workflowDocumentStore.versionData?.description ?? null, - }); - if (settings) { - workflowDocumentStore.mergeSettings(settings); - } + if (!(response.updatedIds ?? []).includes(workflowId)) { + throw new Error( + i18n.baseText('workflowSettings.toggleMCP.updateSkippedError', { + interpolate: { workflowId }, + }), + ); } - if (workflowsListStore.workflowsById[id]) { - workflowsListStore.workflowsById[id] = { - ...workflowsListStore.workflowsById[id], - settings, - versionId, - }; + + applyAvailableInMCPToLocalStores(workflowId, availableInMCP); + + return response; + } + + /** + * Bulk-toggle MCP availability, scoped by an id list, a project, + * or a folder (+ descendants) + */ + async function toggleWorkflowsMcpAccess( + target: ToggleWorkflowsMcpAccessTarget, + availableInMCP: boolean, + ): Promise { + const response = await toggleWorkflowsMcpAccessApi( + rootStore.restApiContext, + target, + availableInMCP, + ); + + for (const id of response.updatedIds ?? []) { + applyAvailableInMCPToLocalStores(id, availableInMCP); } return response; @@ -151,6 +176,7 @@ export const useMCPStore = defineStore(MCP_STORE, () => { fetchWorkflowsAvailableForMCP, setMcpAccessEnabled, toggleWorkflowMcpAccess, + toggleWorkflowsMcpAccess, currentUserMCPKey, getOrCreateApiKey, generateNewApiKey,