Skip to content

Conversation

@MiloradFilipovic
Copy link
Contributor

@MiloradFilipovic MiloradFilipovic commented Dec 10, 2025

Summary

Adding a new modal that lets users enable mcp access for workflows from the settings page. For this to work, I needed to introduce some additional back-end changes:

  • Update triggerNodeType to accept string arrays so we can filter workflows by multiple triggers. Also renamed it to triggerNodeTypes
  • Update getMany method in the workflows service to accept required scopes as a parameter since for mcp we want only workflows with update permissions
  • To make middleware work with the new /mcp/workflows endpoint, I needed to update the list-query middleware to accept all paths ending with workflows
  • Update availableInMCP filter to treat NULL as false

Screenshot of the new modal:

image

Related Linear tickets, Github issues, and Community forum posts

Closes ADO-4437

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@MiloradFilipovic MiloradFilipovic self-assigned this Dec 10, 2025
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Dec 10, 2025
@bundlemon
Copy link

bundlemon bot commented Dec 10, 2025

BundleMon

Files added (2)
Status Path Size Limits
WASM Dependencies
tree-sitter-bash.wasm
+181.26KB -
WASM Dependencies
tree-sitter.wasm
+74.47KB -

Total files change +255.73KB

Groups added (2)
Status Path Size Limits
**/*.js
+6.03MB -
**/*.css
+237.12KB -

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 20 files

Prompt for AI agents (all 7 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/@n8n/db/src/utils/build-workflows-by-nodes-query.ts">

<violation number="1" location="packages/@n8n/db/src/utils/build-workflows-by-nodes-query.ts:38">
P2: Tests may need updating to match new column quoting behavior. The `quoteColumnRef` function will now output `&quot;workflow&quot;.&quot;nodes&quot;` (PostgreSQL/SQLite) or `` `workflow`.`nodes` `` (MySQL), but existing tests expect unquoted `workflow.nodes`. Consider updating test assertions to expect quoted column references.</violation>
</file>

<file name="packages/frontend/editor-ui/src/features/ai/mcpAccess/components/WorkflowLocation.vue">

<violation number="1" location="packages/frontend/editor-ui/src/features/ai/mcpAccess/components/WorkflowLocation.vue:72">
P3: The `$style[&#39;project-link&#39;]` class is referenced but never defined in the CSS module. This is dead code that should be removed from the class binding.</violation>
</file>

<file name="packages/frontend/@n8n/design-system/src/components/N8nNotice/Notice.vue">

<violation number="1" location="packages/frontend/@n8n/design-system/src/components/N8nNotice/Notice.vue:46">
P1: Setting `allowedTags: [&#39;ul&#39;, &#39;li&#39;]` replaces the default allowed tags entirely, which means anchor tags (`a`) will be stripped despite having `allowedAttributes` configured for them. This breaks the existing click handler functionality that relies on anchor tags with `data-key` attributes. Consider including `&#39;a&#39;` in the allowed tags or extending the defaults like done in `n8n-html.ts`.</violation>
</file>

<file name="packages/frontend/editor-ui/src/features/ai/mcpAccess/components/MCPWorkflowsSelect.vue">

<violation number="1" location="packages/frontend/editor-ui/src/features/ai/mcpAccess/components/MCPWorkflowsSelect.vue:44">
P2: Potential race condition: multiple rapid searches can cause loading state to become out of sync. Each search schedules its own setTimeout in the finally block, so an earlier search&#39;s timeout can prematurely set `isLoading = false` while a later request is still in flight. Consider clearing the previous timeout when starting a new search.</violation>
</file>

<file name="packages/cli/src/middlewares/list-query/filter.ts">

<violation number="1" location="packages/cli/src/middlewares/list-query/filter.ts:22">
P2: The `req.path.endsWith(&#39;workflows&#39;)` condition was added here but not to the related `select.ts` and `sort-by.ts` middlewares. Since all three are used together via `listQueryMiddleware`, the MCP workflows endpoint will have filter support but select/sort-by may not function as expected. Consider applying the same condition to `select.ts:18` and `sort-by.ts:20` for consistency.</violation>
</file>

<file name="packages/frontend/editor-ui/src/features/ai/mcpAccess/components/tabs/WorkflowsTable.vue">

<violation number="1" location="packages/frontend/editor-ui/src/features/ai/mcpAccess/components/tabs/WorkflowsTable.vue:99">
P1: Rule violated: **Tests**

The existing tests will fail due to behavior and data-testid changes. The empty state button test expects `router.push` but now emits `connectWorkflows` event. Location tests expect `mcp-workflow-*` data-testids but `WorkflowLocation` component uses `workflow-location-*` ids. Tests must be updated to reflect these changes.</violation>
</file>

<file name="packages/cli/src/modules/mcp/mcp.settings.controller.ts">

<violation number="1" location="packages/cli/src/modules/mcp/mcp.settings.controller.ts:71">
P1: Rule violated: **Tests**

The new `GET /mcp/workflows` endpoint (`getMcpEligibleWorkflows`) lacks test coverage. This is core API functionality with specific filtering logic and permission requirements that should be tested to ensure correct behavior.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

@blacksmith-sh

This comment has been minimized.

@currents-bot
Copy link

currents-bot bot commented Dec 10, 2025

E2E Tests: n8n tests passed after 8m 5.9s

🟢 583 · 🔴 0 · ⚪️ 27 · 🟣 3

View Run Details

Run Details

  • Project: n8n

  • Groups: 2

  • Framework: Playwright

  • Run Status: Passed

  • Commit: 71f1edc

  • Spec files: 130

  • Overall tests: 610

  • Duration: 8m 5.9s

  • Parallelization: 9

Groups

GroupId Results Spec Files Progress
multi-main:ui:isolated 🟢 57 · 🔴 0 · ⚪️ 0 9 / 9
multi-main:ui 🟢 526 · 🔴 0 · ⚪️ 27 · 🟣 3 121 / 121


This message was posted automatically by currents.dev | Integration Settings

@blacksmith-sh

This comment has been minimized.

@blacksmith-sh

This comment has been minimized.

@blacksmith-sh

This comment has been minimized.

@blacksmith-sh

This comment has been minimized.

Copy link
Contributor

@RicardoE105 RicardoE105 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and tested it locally and everything seem to work fine. 🚀

includeScopes?: boolean,
includeFolders?: boolean,
onlySharedWithMe?: boolean,
requiredScopes: Scope[] = ['workflow:read'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick(non-blocking): Should we document this so when the people the the method the know what requiredScopes is 🤔 ?

@github-actions
Copy link
Contributor

@MiloradFilipovic MiloradFilipovic merged commit a84b0e5 into master Dec 15, 2025
42 checks passed
@MiloradFilipovic MiloradFilipovic deleted the ADO-4437-connect-workflows-from-mcp-page branch December 15, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants