ai.mcptools (new) MCPGateway component to connect appmixer-mcp server#594
ai.mcptools (new) MCPGateway component to connect appmixer-mcp server#594DavidDurman wants to merge 7 commits intoAppmixer-ai:devfrom
Conversation
## Walkthrough
This change introduces a new `appmixer.ai.mcptools` module, providing infrastructure for AI assistant tool orchestration within Appmixer flows. It includes gateway logic for tool registration, invocation, and discovery, Redis-based messaging utilities, HTTP/SSE API endpoints for real-time events, and supporting configuration and metadata files.
## Changes
| Files/Groups | Change Summary |
|----------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| MCPGateway.js, component.json | Implements the MCPGateway tool orchestration logic and its component interface for Appmixer flows. |
| bundle.json, module.json, package.json | Adds bundle, module, and package metadata for the new `appmixer.ai.mcptools` module. |
| lib.js | Provides a utility for establishing Redis connections with support for standalone and sentinel modes. |
| plugin.js | Initializes the MCP tools plugin, manages global Redis connections, and loads HTTP/SSE route handlers. |
| routes.js | Implements HTTP API and SSE endpoints for gateway management and real-time event streaming using Redis pub/sub. |
## Possibly related PRs
- clientIO/appmixer-connectors#556: Adds a specific MCP server implementation for Brave Search with tool listing and calling capabilities; related conceptually to the generic MCPGateway orchestration introduced here but no direct code overlap.
## Suggested reviewers
- jirihofmanWarning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/appmixer/ai/mcptools/plugin.js (2)
1-1: Remove redundant "use strict" directive.Static analysis correctly identifies that the "use strict" directive is redundant in JavaScript modules, as they are automatically in strict mode.
-'use strict'; - module.exports = async context => {
19-22: Implement TODO: Remove existing listeners to prevent duplicates.The TODO comment indicates a potential issue with duplicate message handlers but provides no implementation. This could lead to multiple event handlers being registered.
} else { // Make sure listeners are removed so that the ones registered in routes.js // will not introduce duplicate message handlers. - // TODO: only listenrs that are registered by this plugin should be removed. + process.CONNECTOR_STREAM_SUB_CLIENT.removeAllListeners(); }Do you want me to help implement a more targeted solution for removing only specific listeners?
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (1)
64-66: Consider using optional chaining for cleaner code.The current code is safe, but you could use optional chaining for a more concise approach.
Apply this diff to use optional chaining:
- Object.keys(sources || {}).forEach((inPort) => { + Object.keys(sources ?? {}).forEach((inPort) => {This change would apply to both occurrences (lines 64 and 137).
Also applies to: 137-139
src/appmixer/ai/mcptools/MCPGateway/component.json (2)
4-4: Description reads awkwardly – consider re-phrasing for clarityThe current wording is hard to parse (“Define your MCP Server tools that connect to the Appmixer MCP Server which you can install to your LLM clients such as Claude.”).
A crisper alternative:-"description": "Define your MCP Server tools that connect to the Appmixer MCP Server which you can install to your LLM clients such as Claude.", +"description": "Gateway for registering and invoking MCP-Server tools from LLM clients (e.g. Claude) via Appmixer.",
17-17: Consider externalising the base64 iconInlining the entire PNG inflates the JSON (~300 KB) and makes diffs noisy.
Storingicon.pngnext to the component and referencing it:-"icon": "data:image/png;base64,...." +"icon": "./icon.png"keeps the manifest readable and Git-friendly while still being bundled by the build system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/appmixer/ai/mcptools/MCPGateway/icon.svgis excluded by!**/*.svgsrc/appmixer/ai/mcptools/icon.pngis excluded by!**/*.png
📒 Files selected for processing (8)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js(1 hunks)src/appmixer/ai/mcptools/MCPGateway/component.json(1 hunks)src/appmixer/ai/mcptools/bundle.json(1 hunks)src/appmixer/ai/mcptools/lib.js(1 hunks)src/appmixer/ai/mcptools/module.json(1 hunks)src/appmixer/ai/mcptools/package.json(1 hunks)src/appmixer/ai/mcptools/plugin.js(1 hunks)src/appmixer/ai/mcptools/routes.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
`src/appmixer/**/bundle.json`: The bundle.json file must follow the specified schema, including name (lower case, appmixer.${CONNECTOR_NAME}), version (default 1.0.0), and changelo...
src/appmixer/**/bundle.json: The bundle.json file must follow the specified schema, including name (lower case, appmixer.${CONNECTOR_NAME}), version (default 1.0.0), and changelog (object describing changes).
src/appmixer/ai/mcptools/bundle.json
`**/*/bundle.json`: Make sure `version` also matches the last entry in `changelog`
**/*/bundle.json: Make sureversionalso matches the last entry inchangelog
src/appmixer/ai/mcptools/bundle.json
`src/appmixer/**/package.json`: Not using lock file. All dependencies should be fixed
src/appmixer/**/package.json: Not using lock file. All dependencies should be fixed
src/appmixer/ai/mcptools/package.json
`src/appmixer/**/*.js`: Not necessary to add try/catch blocks in their `receive` function. Appmixer engine automatically handles any exceptions that originate in these async functi...
src/appmixer/**/*.js: Not necessary to add try/catch blocks in theirreceivefunction. Appmixer engine automatically handles any exceptions that originate in these async functions.The directive
"use strict";is desirable here
src/appmixer/ai/mcptools/lib.jssrc/appmixer/ai/mcptools/plugin.jssrc/appmixer/ai/mcptools/MCPGateway/MCPGateway.jssrc/appmixer/ai/mcptools/routes.js
🪛 Biome (1.9.4)
src/appmixer/ai/mcptools/plugin.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 139-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (8)
src/appmixer/ai/mcptools/lib.js (1)
5-43: Add error handling for Redis connection.The function lacks proper error handling for Redis connection failures, which could cause the application to crash.
connectRedis: async function() { + try { const connection = { uri: process.env.REDIS_URI, mode: process.env.REDIS_MODE || 'standalone', sentinels: process.env.REDIS_SENTINELS, sentinelMasterName: process.env.REDIS_SENTINEL_MASTER_NAME, sentinelRedisPassword: process.env.REDIS_SENTINEL_PASSWORD, enableTLSForSentinelMode: process.env.REDIS_SENTINEL_ENABLE_TLS, caPath: process.env.REDIS_CA_PATH, useSSL: process.env.REDIS_USE_SSL === 'true' || parseInt(process.env.REDIS_USE_SSL) > 0 }; const options = {}; if (connection.useSSL) { options.tls = { ca: connection.caPath ? await fs.readFile(connection.caPath) : undefined }; } let client; if (connection.mode === 'replica' && connection.sentinels) { const sentinelsArray = connection.sentinels.split(','); client = new Redis({ sentinels: sentinelsArray, name: connection.sentinelMasterName, ...(connection.sentinelRedisPassword ? { password: connection.sentinelRedisPassword } : {}), ...(connection.enableTLSForSentinelMode ? { enableTLSForSentinelMode: connection.enableTLSForSentinelMode } : {}) }); } else { client = connection.uri ? new Redis(connection.uri, options) : new Redis(); } return client; + } catch (error) { + throw new Error(`Failed to connect to Redis: ${error.message}`); + } }Likely an incorrect or invalid review comment.
src/appmixer/ai/mcptools/package.json (1)
1-10: LGTM! Dependencies properly fixed.The package.json follows the coding guidelines with all dependencies using fixed versions as required for
src/appmixer/**/package.jsonfiles.src/appmixer/ai/mcptools/bundle.json (1)
1-9: LGTM! Bundle configuration follows schema requirements.The bundle.json correctly follows the specified schema with lowercase name, proper version, and matching changelog entry as required by coding guidelines.
src/appmixer/ai/mcptools/module.json (1)
1-10: LGTM! Module metadata is properly structured.The module.json file contains appropriate metadata for the MCP Tools module with correct categorization and description.
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (2)
68-82: Good validation logic with clear error messages.The validation ensures proper component types are connected to each port and provides helpful error messages to guide users. The use of
context.CancelErroris appropriate here.Also applies to: 141-153
272-282: Excellent error handling approach.Catching errors and returning them as messages instead of letting them propagate prevents unwanted Appmixer component retries, which is the correct behavior for tool invocation failures.
src/appmixer/ai/mcptools/routes.js (1)
116-151: Well-implemented SSE connection handling.The implementation correctly:
- Disables compression to avoid chunked encoding issues
- Sends heartbeat pings to maintain connection through proxies
- Properly cleans up resources on disconnect
- Uses PassThrough stream for efficient data forwarding
src/appmixer/ai/mcptools/MCPGateway/component.json (1)
8-13: Double-check thatoptionsinside an out-port is supported by the runtimeMost Appmixer component specs put field declarations either in
inPortsor in the component-levelsettings, not inoutPorts.
If the engine does not recognise this pattern, the UI may silently ignore the field and the port will emit an empty object.Action: verify that
optionson an output port renders in the designer and that runtime messages contain the"prompt"field.
If not, move the schema to a dedicated output payload or to a regular input port.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/appmixer/ai/mcptools/lib.js (2)
1-1: Add the "use strict" directive.Per coding guidelines, the "use strict" directive is desirable for files matching
src/appmixer/**/*.js.
1-1: Missing fs import for readFile operation.The code calls
fs.readFile()on line 21 but thefsmodule is not imported, which will cause a runtime error.+'use strict'; + +const fs = require('fs').promises; const Redis = require('ioredis');
🧹 Nitpick comments (2)
src/appmixer/ai/mcptools/lib.js (2)
31-37: Improve sentinel configuration structure.The sentinel configuration could be more readable and maintainable by extracting the conditional logic.
- client = new Redis({ - sentinels: sentinelsArray, - name: connection.sentinelMasterName, - ...(connection.sentinelRedisPassword ? { password: connection.sentinelRedisPassword } : {}), - ...(connection.enableTLSForSentinelMode ? - { enableTLSForSentinelMode: connection.enableTLSForSentinelMode } : {}) - }); + const sentinelConfig = { + sentinels: sentinelsArray, + name: connection.sentinelMasterName + }; + + if (connection.sentinelRedisPassword) { + sentinelConfig.password = connection.sentinelRedisPassword; + } + + if (connection.enableTLSForSentinelMode) { + sentinelConfig.enableTLSForSentinelMode = connection.enableTLSForSentinelMode; + } + + client = new Redis(sentinelConfig);
5-43: Consider adding JSDoc documentation.The function would benefit from comprehensive documentation explaining its purpose, parameters (via environment variables), and return value.
+ /** + * Creates and configures a Redis client based on environment variables. + * Supports both standalone and sentinel modes with optional SSL/TLS. + * + * Environment variables: + * - REDIS_URI: Redis connection URI + * - REDIS_MODE: Connection mode ('standalone' or 'replica') + * - REDIS_SENTINELS: Comma-separated sentinel addresses + * - REDIS_SENTINEL_MASTER_NAME: Sentinel master name + * - REDIS_SENTINEL_PASSWORD: Sentinel password + * - REDIS_SENTINEL_ENABLE_TLS: Enable TLS for sentinel mode + * - REDIS_CA_PATH: Path to CA certificate file + * - REDIS_USE_SSL: Enable SSL/TLS + * + * @returns {Promise<Redis>} Configured Redis client instance + */ connectRedis: async function() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/appmixer/ai/mcptools/lib.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/appmixer/**/*.js`: Not necessary to add try/catch blocks in their `receive`...
src/appmixer/**/*.js: Not necessary to add try/catch blocks in theirreceivefunction. Appmixer engine automatically handles any exceptions that originate in these async functions.The directive
"use strict";is desirable here
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/appmixer/ai/mcptools/lib.js
🔇 Additional comments (1)
src/appmixer/ai/mcptools/lib.js (1)
39-39: Add error handling for Redis client creation.The Redis client creation should include proper error handling to provide meaningful feedback when connection fails.
- client = connection.uri ? new Redis(connection.uri, options) : new Redis(); + try { + client = connection.uri ? new Redis(connection.uri, options) : new Redis(); + } catch (error) { + throw new Error(`Failed to create Redis client: ${error.message}`); + }Likely an incorrect or invalid review comment.
|
|
||
| if (connection.mode === 'replica' && connection.sentinels) { | ||
|
|
||
| const sentinelsArray = connection.sentinels.split(','); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for sentinel addresses.
The code splits the sentinels string without validating its format, which could lead to runtime errors if the environment variable contains unexpected values.
- const sentinelsArray = connection.sentinels.split(',');
+ const sentinelsArray = connection.sentinels.split(',').map(s => s.trim()).filter(s => s);
+ if (sentinelsArray.length === 0) {
+ throw new Error('REDIS_SENTINELS must contain at least one sentinel address');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sentinelsArray = connection.sentinels.split(','); | |
| const sentinelsArray = connection.sentinels | |
| .split(',') | |
| .map(s => s.trim()) | |
| .filter(s => s); | |
| if (sentinelsArray.length === 0) { | |
| throw new Error('REDIS_SENTINELS must contain at least one sentinel address'); | |
| } |
🤖 Prompt for AI Agents
In src/appmixer/ai/mcptools/lib.js at line 29, the code splits the sentinels
string without validating its format, which may cause runtime errors. Add input
validation to check that connection.sentinels is a non-empty string and that
each sentinel address in the resulting array matches the expected format (e.g.,
host:port). If validation fails, handle the error gracefully by logging a clear
message or throwing an informative error before proceeding.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/appmixer/ai/mcptools/lib.js (1)
32-32: The sentinel validation issue remains unaddressed.Based on past review comments, input validation for sentinel addresses was suggested but not implemented. The code still splits the sentinels string without validation, which could lead to runtime errors if the environment variable contains unexpected values.
Consider adding validation:
- const sentinelsArray = connection.sentinels.split(','); + const sentinelsArray = connection.sentinels.split(',').map(s => s.trim()).filter(s => s); + if (sentinelsArray.length === 0) { + throw new Error('REDIS_SENTINELS must contain at least one sentinel address'); + }
🧹 Nitpick comments (4)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (4)
66-66: Consider using optional chaining for safer property access.The static analysis correctly identifies that this could be simplified with optional chaining to avoid potential runtime errors.
- if (source[agentComponentId] && source[agentComponentId].includes(toolsPort)) { + if (source[agentComponentId]?.includes(toolsPort)) {
139-139: Consider using optional chaining for safer property access.Similar to the previous case, optional chaining would make this code more robust.
- if (source[agentComponentId] && source[agentComponentId].includes(mcpPort)) { + if (source[agentComponentId]?.includes(mcpPort)) {
68-73: Improve error message formatting.The error message contains inconsistent line breaks and formatting that could make it hard to read in logs.
- error = `Component ${componentId} is not of type 'ToolStart' but ${component.type}. - Every tool chain connected to the '${toolsPort}' port of the AI Agent - must start with 'ToolStart' and end with 'ToolOutput'. - This is where you describe what the tool does and what parameters should the AI model provide to it.`; + error = `Component ${componentId} is not of type 'ToolStart' but ${component.type}. Every tool chain connected to the '${toolsPort}' port of the AI Agent must start with 'ToolStart' and end with 'ToolOutput'. This is where you describe what the tool does and what parameters should the AI model provide to it.`;
264-264: Fix typo in comment.There's a typo in the comment: "compoennt" should be "component".
- // Get back the original compoennt UUID back from the short version. + // Get back the original component UUID from the short version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/appmixer/ai/mcptools/MCPGateway/icon.svgis excluded by!**/*.svgsrc/appmixer/ai/mcptools/icon.pngis excluded by!**/*.png
📒 Files selected for processing (8)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js(1 hunks)src/appmixer/ai/mcptools/MCPGateway/component.json(1 hunks)src/appmixer/ai/mcptools/bundle.json(1 hunks)src/appmixer/ai/mcptools/lib.js(1 hunks)src/appmixer/ai/mcptools/module.json(1 hunks)src/appmixer/ai/mcptools/package.json(1 hunks)src/appmixer/ai/mcptools/plugin.js(1 hunks)src/appmixer/ai/mcptools/routes.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/appmixer/ai/mcptools/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/appmixer/ai/mcptools/bundle.json
- src/appmixer/ai/mcptools/module.json
- src/appmixer/ai/mcptools/MCPGateway/component.json
- src/appmixer/ai/mcptools/routes.js
🧰 Additional context used
📓 Path-based instructions (1)
`src/appmixer/**/*.js`: Not necessary to add try/catch blocks in their `receive`...
src/appmixer/**/*.js: Not necessary to add try/catch blocks in theirreceivefunction. Appmixer engine automatically handles any exceptions that originate in these async functions.The directive
"use strict";is desirable here
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.jssrc/appmixer/ai/mcptools/lib.jssrc/appmixer/ai/mcptools/plugin.js
🪛 Biome (1.9.4)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 139-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/appmixer/ai/mcptools/lib.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/ai/mcptools/plugin.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (5)
src/appmixer/ai/mcptools/lib.js (1)
1-1: Keep the 'use strict' directive per coding guidelines.While static analysis suggests the 'use strict' directive is redundant in modules, the coding guidelines specifically state it's "desirable" for files matching
src/appmixer/**/*.js. The coding guidelines should take precedence over static analysis in this case.src/appmixer/ai/mcptools/plugin.js (2)
1-1: Keep the 'use strict' directive per coding guidelines.Consistent with the previous file, the coding guidelines specify that 'use strict' is desirable for
src/appmixer/**/*.jsfiles, so it should be retained despite the static analysis warning.
11-19: Add error handling for Redis connection failures.If the Redis connection fails, the plugin initialization should handle the error gracefully rather than allowing it to propagate and potentially crash the application.
- if (!process.CONNECTOR_STREAM_PUB_CLIENT) { - context.log('info', '[AI.MCPTOOLS] Connecting Redis Publisher client.'); - process.CONNECTOR_STREAM_PUB_CLIENT = await lib.connectRedis(); - context.log('info', '[AI.MCPTOOLS] Redis Publisher client connected.'); - } + if (!process.CONNECTOR_STREAM_PUB_CLIENT) { + try { + context.log('info', '[AI.MCPTOOLS] Connecting Redis Publisher client.'); + process.CONNECTOR_STREAM_PUB_CLIENT = await lib.connectRedis(); + context.log('info', '[AI.MCPTOOLS] Redis Publisher client connected.'); + } catch (err) { + context.log('error', '[AI.MCPTOOLS] Failed to connect Redis Publisher client:', err.message); + throw err; + } + }Likely an incorrect or invalid review comment.
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (2)
1-1: Keep the 'use strict' directive per coding guidelines.Consistent with the other files, the coding guidelines specify that 'use strict' is desirable for
src/appmixer/**/*.jsfiles.
261-266: Add validation for function arguments parsing.The JSON parsing of function arguments could fail if the arguments string is malformed, causing the entire tool call to fail.
- const args = typeof req.data.function.arguments === 'string' ? JSON.parse(req.data.function.arguments) : req.data.function.arguments; + let args; + try { + args = typeof req.data.function.arguments === 'string' ? JSON.parse(req.data.function.arguments) : req.data.function.arguments; + } catch (err) { + await context.log({ step: 'parse-arguments-error', arguments: req.data.function.arguments, err }); + return context.response(`Error parsing function arguments: ${err.message}`, 400); + }Likely an incorrect or invalid review comment.
src/appmixer/ai/mcptools/lib.js
Outdated
| const options = {}; | ||
| if (connection.useSSL) { | ||
| options.tls = { | ||
| ca: connection.caPath ? await fs.readFile(connection.caPath) : undefined |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for certificate file reading.
The fs.readFile() call could fail if the certificate file doesn't exist or isn't readable, which would cause the Redis connection to fail silently or with unclear errors.
- ca: connection.caPath ? await fs.readFile(connection.caPath) : undefined
+ ca: connection.caPath ? await fs.readFile(connection.caPath).catch(err => {
+ throw new Error(`Failed to read Redis CA certificate from ${connection.caPath}: ${err.message}`);
+ }) : undefined📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ca: connection.caPath ? await fs.readFile(connection.caPath) : undefined | |
| ca: connection.caPath | |
| ? await fs | |
| .readFile(connection.caPath) | |
| .catch(err => { | |
| throw new Error( | |
| `Failed to read Redis CA certificate from ${connection.caPath}: ${err.message}` | |
| ); | |
| }) | |
| : undefined |
🤖 Prompt for AI Agents
In src/appmixer/ai/mcptools/lib.js at line 24, the fs.readFile call for reading
the certificate file lacks error handling, which can cause silent failures if
the file is missing or unreadable. Wrap the await fs.readFile(connection.caPath)
call in a try-catch block to catch and handle any errors, such as logging the
error or throwing a descriptive exception, ensuring the Redis connection failure
is clear and manageable.
| } else { | ||
| // Make sure listeners are removed so that the ones registered in routes.js | ||
| // will not introduce duplicate message handlers. | ||
| // TODO: only listenrs that are registered by this plugin should be removed. | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complete the TODO or implement proper listener management.
The TODO comment indicates incomplete functionality for removing existing listeners. This could lead to duplicate message handlers being registered, which might cause issues with message processing or memory leaks.
Either implement the listener removal logic or consider a different approach:
} else {
- // Make sure listeners are removed so that the ones registered in routes.js
- // will not introduce duplicate message handlers.
- // TODO: only listenrs that are registered by this plugin should be removed.
+ // Remove all existing listeners to prevent duplicates
+ process.CONNECTOR_STREAM_SUB_CLIENT.removeAllListeners();
}Alternatively, you could track which listeners were added by this plugin specifically and only remove those.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| // Make sure listeners are removed so that the ones registered in routes.js | |
| // will not introduce duplicate message handlers. | |
| // TODO: only listenrs that are registered by this plugin should be removed. | |
| } | |
| } else { | |
| // Remove all existing listeners to prevent duplicates | |
| process.CONNECTOR_STREAM_SUB_CLIENT.removeAllListeners(); | |
| } |
🤖 Prompt for AI Agents
In src/appmixer/ai/mcptools/plugin.js around lines 20 to 24, the TODO indicates
incomplete listener removal logic that risks duplicate handlers. Implement logic
to track listeners added by this plugin, such as storing references when adding
them, and then remove only those specific listeners here to avoid removing
unrelated ones. This ensures proper cleanup and prevents duplicate message
handlers or memory leaks.
| while ((Date.now() - pollStart) < pollTimeout) { | ||
| const result = await context.flow.stateGet(toolCall.id); | ||
| if (result) { | ||
| await context.flow.stateUnset(toolCall.id); | ||
| return result.output; | ||
| } | ||
| // Sleep. | ||
| await new Promise((resolve) => setTimeout(resolve, pollInterval)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential race condition in polling loop.
The polling loop could potentially miss results if the state is set and unset between checks, and there's no protection against multiple concurrent polls for the same tool call.
Consider adding a flag to prevent concurrent polling:
+ const pollKey = `polling:${toolCall.id}`;
+ if (await context.flow.stateGet(pollKey)) {
+ return 'Error: Tool call already being processed.';
+ }
+ await context.flow.stateSet(pollKey, true);
+
const pollStart = Date.now();
const pollTimeout = context.config.TOOLS_OUTPUT_POLL_TIMEOUT || TOOLS_OUTPUT_POLL_TIMEOUT;
const pollInterval = context.config.TOOLS_OUTPUT_POLL_INTERVAL || TOOLS_OUTPUT_POLL_INTERVAL;
while ((Date.now() - pollStart) < pollTimeout) {
const result = await context.flow.stateGet(toolCall.id);
if (result) {
await context.flow.stateUnset(toolCall.id);
+ await context.flow.stateUnset(pollKey);
return result.output;
}
// Sleep.
await new Promise((resolve) => setTimeout(resolve, pollInterval));
}
+ await context.flow.stateUnset(pollKey);
return 'Error: Tool timed out.';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while ((Date.now() - pollStart) < pollTimeout) { | |
| const result = await context.flow.stateGet(toolCall.id); | |
| if (result) { | |
| await context.flow.stateUnset(toolCall.id); | |
| return result.output; | |
| } | |
| // Sleep. | |
| await new Promise((resolve) => setTimeout(resolve, pollInterval)); | |
| } | |
| const pollKey = `polling:${toolCall.id}`; | |
| if (await context.flow.stateGet(pollKey)) { | |
| return 'Error: Tool call already being processed.'; | |
| } | |
| await context.flow.stateSet(pollKey, true); | |
| const pollStart = Date.now(); | |
| const pollTimeout = context.config.TOOLS_OUTPUT_POLL_TIMEOUT || TOOLS_OUTPUT_POLL_TIMEOUT; | |
| const pollInterval = context.config.TOOLS_OUTPUT_POLL_INTERVAL || TOOLS_OUTPUT_POLL_INTERVAL; | |
| while ((Date.now() - pollStart) < pollTimeout) { | |
| const result = await context.flow.stateGet(toolCall.id); | |
| if (result) { | |
| await context.flow.stateUnset(toolCall.id); | |
| await context.flow.stateUnset(pollKey); | |
| return result.output; | |
| } | |
| // Sleep. | |
| await new Promise((resolve) => setTimeout(resolve, pollInterval)); | |
| } | |
| await context.flow.stateUnset(pollKey); | |
| return 'Error: Tool timed out.'; |
🤖 Prompt for AI Agents
In src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js around lines 242 to 250,
the polling loop may miss results due to a race condition and allows multiple
concurrent polls for the same tool call. To fix this, introduce a flag or a
tracking mechanism to prevent concurrent polling for the same toolCall.id.
Before starting the poll, check if a poll is already in progress for that id and
skip or wait accordingly, ensuring only one poll runs per tool call at a time.
| }; | ||
| }); | ||
| let toolName = (component.label || component.type.split('.').pop()); | ||
| toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, 64 - componentId.length - 1); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with tool name length calculation.
The tool name length limit calculation may not account for the actual componentId length correctly, potentially causing names to exceed the 64-character limit.
- toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, 64 - componentId.length - 1);
+ const maxNameLength = Math.max(1, 64 - componentId.length - 1);
+ toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, maxNameLength);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, 64 - componentId.length - 1); | |
| const maxNameLength = Math.max(1, 64 - componentId.length - 1); | |
| toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, maxNameLength); |
🤖 Prompt for AI Agents
In src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js at line 208, the
calculation for slicing the toolName to fit within the 64-character limit may
not correctly account for the length of componentId. Adjust the slicing logic to
ensure the total length of the resulting toolName plus componentId and the
separator does not exceed 64 characters by accurately subtracting the length of
componentId and the separator from 64 before slicing.
|
|
||
| mcpListTools: function(context, componentId) { | ||
|
|
||
| return context.callAppmixer({ |
There was a problem hiding this comment.
use the httpRequest instead, the same as in this PR #588 to ensure compatibility with engine versions earlier than 6.1.
| } | ||
| const category = component.type.split('.').slice(0, 2).join('.'); | ||
| const type = component.type.split('.').at(-1); | ||
| if (category === 'appmixer.mcpservers' && type === 'MCPServer') { |
There was a problem hiding this comment.
return (category === 'appmixer.mcpservers' && type === 'MCPServer') ;)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
src/appmixer/ai/mcptools/lib.js (2)
24-24: Add error handling for certificate file reading.The
fs.readFile()call could fail if the certificate file doesn't exist or isn't readable, which would cause the Redis connection to fail silently or with unclear errors.- ca: connection.caPath ? await fs.readFile(connection.caPath) : undefined + ca: connection.caPath ? await fs.readFile(connection.caPath).catch(err => { + throw new Error(`Failed to read Redis CA certificate from ${connection.caPath}: ${err.message}`); + }) : undefined
32-32: Add input validation for sentinel addresses.The code splits the sentinels string without validating its format, which could lead to runtime errors if the environment variable contains unexpected values.
- const sentinelsArray = connection.sentinels.split(','); + const sentinelsArray = connection.sentinels.split(',').map(s => s.trim()).filter(s => s); + if (sentinelsArray.length === 0) { + throw new Error('REDIS_SENTINELS must contain at least one sentinel address'); + }src/appmixer/ai/mcptools/plugin.js (1)
20-24: Complete the TODO or implement proper listener management.The TODO comment indicates incomplete functionality for removing existing listeners. This could lead to duplicate message handlers being registered, which might cause issues with message processing or memory leaks.
Either implement the listener removal logic or consider a different approach:
} else { - // Make sure listeners are removed so that the ones registered in routes.js - // will not introduce duplicate message handlers. - // TODO: only listenrs that are registered by this plugin should be removed. + // Remove all existing listeners to prevent duplicates + process.CONNECTOR_STREAM_SUB_CLIENT.removeAllListeners(); }Alternatively, you could track which listeners were added by this plugin specifically and only remove those.
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (4)
117-117: Simplify boolean expression.
return (category === 'appmixer.mcpservers' && type === 'MCPServer');)
90-94: Use httpRequest instead of callAppmixer for compatibility.Use the
httpRequestinstead, the same as in this PR #588 to ensure compatibility with engine versions earlier than 6.1.
208-208: Potential issue with tool name length calculation.The tool name length limit calculation may not account for the actual componentId length correctly, potentially causing names to exceed the 64-character limit.
- toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, 64 - componentId.length - 1); + const maxNameLength = Math.max(1, 64 - componentId.length - 1); + toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, maxNameLength);
242-250: Potential race condition in polling loop.The polling loop could potentially miss results if the state is set and unset between checks, and there's no protection against multiple concurrent polls for the same tool call.
Consider adding a flag to prevent concurrent polling:
+ const pollKey = `polling:${toolCall.id}`; + if (await context.flow.stateGet(pollKey)) { + return 'Error: Tool call already being processed.'; + } + await context.flow.stateSet(pollKey, true); + const pollStart = Date.now(); const pollTimeout = context.config.TOOLS_OUTPUT_POLL_TIMEOUT || TOOLS_OUTPUT_POLL_TIMEOUT; const pollInterval = context.config.TOOLS_OUTPUT_POLL_INTERVAL || TOOLS_OUTPUT_POLL_INTERVAL; while ((Date.now() - pollStart) < pollTimeout) { const result = await context.flow.stateGet(toolCall.id); if (result) { await context.flow.stateUnset(toolCall.id); + await context.flow.stateUnset(pollKey); return result.output; } // Sleep. await new Promise((resolve) => setTimeout(resolve, pollInterval)); } + await context.flow.stateUnset(pollKey); return 'Error: Tool timed out.';
🧹 Nitpick comments (2)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (2)
66-66: Use optional chaining for safer property access.Static analysis suggests using optional chaining to safely access nested properties.
- if (source[agentComponentId] && source[agentComponentId].includes(toolsPort)) { + if (source[agentComponentId]?.includes(toolsPort)) {
139-139: Use optional chaining for safer property access.Static analysis suggests using optional chaining to safely access nested properties.
- if (source[agentComponentId] && source[agentComponentId].includes(mcpPort)) { + if (source[agentComponentId]?.includes(mcpPort)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/appmixer/ai/mcptools/MCPGateway/icon.svgis excluded by!**/*.svgsrc/appmixer/ai/mcptools/icon.pngis excluded by!**/*.png
📒 Files selected for processing (8)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js(1 hunks)src/appmixer/ai/mcptools/MCPGateway/component.json(1 hunks)src/appmixer/ai/mcptools/bundle.json(1 hunks)src/appmixer/ai/mcptools/lib.js(1 hunks)src/appmixer/ai/mcptools/module.json(1 hunks)src/appmixer/ai/mcptools/package.json(1 hunks)src/appmixer/ai/mcptools/plugin.js(1 hunks)src/appmixer/ai/mcptools/routes.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/appmixer/ai/mcptools/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/appmixer/ai/mcptools/bundle.json
- src/appmixer/ai/mcptools/module.json
- src/appmixer/ai/mcptools/MCPGateway/component.json
🧰 Additional context used
📓 Path-based instructions (3)
`src/appmixer/**`: Use `src/appmixer` for source code of connectors.
src/appmixer/**: Usesrc/appmixerfor source code of connectors.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
src/appmixer/ai/mcptools/routes.jssrc/appmixer/ai/mcptools/MCPGateway/MCPGateway.jssrc/appmixer/ai/mcptools/lib.jssrc/appmixer/ai/mcptools/plugin.js
`**/*.js`: Add one empty line after function definition. Use 4 spaces for indentation.
**/*.js: Add one empty line after function definition.
Use 4 spaces for indentation.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
src/appmixer/ai/mcptools/routes.jssrc/appmixer/ai/mcptools/MCPGateway/MCPGateway.jssrc/appmixer/ai/mcptools/lib.jssrc/appmixer/ai/mcptools/plugin.js
`src/appmixer/**/*.js`: Not necessary to add try/catch blocks in their `receive`...
src/appmixer/**/*.js: Not necessary to add try/catch blocks in theirreceivefunction. Appmixer engine automatically handles any exceptions that originate in these async functions.The directive
"use strict";is desirable here
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/appmixer/ai/mcptools/routes.jssrc/appmixer/ai/mcptools/MCPGateway/MCPGateway.jssrc/appmixer/ai/mcptools/lib.jssrc/appmixer/ai/mcptools/plugin.js
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/** : Use `src/appmixer` for source code of connectors.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/*/{service.json,auth.js,bundle.json} : Connectors consist of service.json, auth.js, and bundle.json files, located in the `src/appmixer` folder.
src/appmixer/ai/mcptools/routes.js (11)
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#494
File: src/appmixer/ai/voyageai/CreateTextEmbeddingsFromFile/CreateTextEmbeddingsFromFile.js:36-40
Timestamp: 2025-04-30T12:27:08.628Z
Learning: In the appmixer-connectors codebase, `context.httpRequest` is an axios instance, so error handling should follow axios patterns (using try/catch blocks) rather than checking for `response.ok` which is a Fetch API pattern.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/GetUser/GetUser.js:12-14
Timestamp: 2025-04-28T11:22:40.693Z
Learning: Error handling for GitHub API requests in the appmixer connectors is already implemented at a different level (likely in lib.apiRequest), so additional try-catch blocks aren't needed in individual component files.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#444
File: src/appmixer/airtable/records/CreateOrUpdateRecord/CreateOrUpdateRecord.js:31-37
Timestamp: 2025-03-28T16:21:57.663Z
Learning: Error handling for HTTP requests in Airtable connector components is already handled at the component framework level, so additional try/catch blocks around context.httpRequest calls are not needed.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/ListPullRequests/ListPullRequests.js:19-19
Timestamp: 2025-04-28T12:19:17.612Z
Learning: The team prefers not to add explicit try/catch error handling around API requests in the GitHub connector components. Future code reviews should not suggest adding error handling blocks to these components.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#461
File: src/appmixer/microsoft/sharepoint/FindFilesOrFolders/FindFilesOrFolders.js:0-0
Timestamp: 2025-04-22T10:39:55.176Z
Learning: The `makeRequest()` function imported from '../common' in the Microsoft SharePoint connector components already has built-in error handling, making additional try-catch blocks around its calls unnecessary.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#461
File: src/appmixer/microsoft/sharepoint/FindFilesOrFolders/FindFilesOrFolders.js:0-0
Timestamp: 2025-04-22T10:39:55.176Z
Learning: The `makeRequest()` function imported from '../common' in the Microsoft SharePoint connector components already incorporates error handling through the `commons.formatError()` wrapper, making additional try-catch blocks around its calls redundant.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#518
File: src/appmixer/canva/core/CreateDesign/CreateDesign.js:27-38
Timestamp: 2025-06-05T13:14:13.495Z
Learning: In the appmixer codebase for Canva connectors, try-catch blocks are not needed for HTTP requests in component files - the framework handles errors appropriately without explicit error handling.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#518
File: src/appmixer/canva/core/MakeApiCall/MakeApiCall.js:16-18
Timestamp: 2025-06-05T13:13:44.676Z
Learning: In the Appmixer Canva integration components, try-catch error handling around JSON.parse() calls is not preferred by the user.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/FindPullRequest/FindPullRequest.js:24-24
Timestamp: 2025-04-28T12:21:27.785Z
Learning: Try-catch blocks aren't needed around lib.apiRequest() calls in the GitHub connector components as error handling is likely managed elsewhere in the codebase.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#428
File: src/appmixer/freshdesk/tickets/DeletedTicket/DeletedTicket.js:30-37
Timestamp: 2025-03-13T16:32:39.694Z
Learning: In the appmixer-connectors repository, explicit try/catch blocks may not be needed around API calls in component implementations as error handling might be managed at a higher level in the framework.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/*/{service.json,auth.js,bundle.json} : Connectors consist of service.json, auth.js, and bundle.json files, located in the `src/appmixer` folder.
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (10)
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#490
File: src/appmixer/ai/claude/AIAgent/AIAgent.js:70-72
Timestamp: 2025-05-06T10:06:04.289Z
Learning: In the AIAgent component, `toolUse.name` follows the format 'function_' + componentId where componentId is a UUID, so extracting it with `toolUse.name.split('_')[1]` is the correct approach.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#490
File: src/appmixer/ai/claude/AIAgent/AIAgent.js:70-72
Timestamp: 2025-05-06T10:06:04.289Z
Learning: In the AIAgent component, `toolUse.name` follows the format 'function_' + componentId where componentId is a UUID, so extracting it with `toolUse.name.split('_')[1]` is the correct approach.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/FindIssue/FindIssue.js:14-18
Timestamp: 2025-04-28T11:43:24.482Z
Learning: The FindIssue.js component in the GitHub connector should use encodeURIComponent when constructing search query strings to ensure special characters in repositoryId, title, and labels don't break the URL structure when making API requests.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.822Z
Learning: Applies to src/appmixer/*/core/*/component.json : The `component.json` file's `name` property must match the pattern ^[\w]+\.[\w]+\.[\w]+\.[\w]+$.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#494
File: src/appmixer/ai/voyageai/CreateTextEmbeddingsFromFile/CreateTextEmbeddingsFromFile.js:36-40
Timestamp: 2025-04-30T12:27:08.628Z
Learning: In the appmixer-connectors codebase, `context.httpRequest` is an axios instance, so error handling should follow axios patterns (using try/catch blocks) rather than checking for `response.ok` which is a Fetch API pattern.
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is widely used across the appmixer-connectors codebase, including in commons components of multiple connectors. While some files are being updated to use context.httpRequest instead, the dependency needs to remain in package.json files for components that still use it.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#518
File: src/appmixer/canva/core/CreateDesign/CreateDesign.js:27-38
Timestamp: 2025-06-05T13:14:13.495Z
Learning: In the appmixer codebase for Canva connectors, try-catch blocks are not needed for HTTP requests in component files - the framework handles errors appropriately without explicit error handling.
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is still used in commons components of the Jira connector, and should be kept in package.json even though it was removed from auth.js.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#444
File: src/appmixer/airtable/records/CreateOrUpdateRecord/CreateOrUpdateRecord.js:31-37
Timestamp: 2025-03-28T16:21:57.663Z
Learning: Error handling for HTTP requests in Airtable connector components is already handled at the component framework level, so additional try/catch blocks around context.httpRequest calls are not needed.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/GetUser/GetUser.js:12-14
Timestamp: 2025-04-28T11:22:40.693Z
Learning: Error handling for GitHub API requests in the appmixer connectors is already implemented at a different level (likely in lib.apiRequest), so additional try-catch blocks aren't needed in individual component files.
src/appmixer/ai/mcptools/lib.js (13)
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is widely used across the appmixer-connectors codebase, including in commons components of multiple connectors. While some files are being updated to use context.httpRequest instead, the dependency needs to remain in package.json files for components that still use it.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/*/{service.json,auth.js,bundle.json} : Connectors consist of service.json, auth.js, and bundle.json files, located in the `src/appmixer` folder.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.822Z
Learning: Applies to src/appmixer/*/auth.js : The `auth.js` file for a connector must export an object with a `type` property set to either 'apiKey', 'pwd', or 'oauth2', and provide the required authentication definition structure.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.822Z
Learning: Applies to src/appmixer/*/core/*/*.js : When adding a new field to a component, update the corresponding behavior JS file to handle the new field, especially in `context.httpRequest` calls.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to test/utils.js : Use `test/utils.js` for Appmixer stub.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/** : Use `src/appmixer` for source code of connectors.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.822Z
Learning: Applies to src/appmixer/*/core/*/*.js : Do not check for required properties in the `receive` function of behavior files; required properties are validated by the input schema in component.json.
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is still used in commons components of the Jira connector, and should be kept in package.json even though it was removed from auth.js.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#494
File: src/appmixer/ai/voyageai/CreateTextEmbeddingsFromFile/CreateTextEmbeddingsFromFile.js:36-40
Timestamp: 2025-04-30T12:27:08.628Z
Learning: In the appmixer-connectors codebase, `context.httpRequest` is an axios instance, so error handling should follow axios patterns (using try/catch blocks) rather than checking for `response.ok` which is a Fetch API pattern.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/GetUser/GetUser.js:12-14
Timestamp: 2025-04-28T11:22:40.693Z
Learning: Error handling for GitHub API requests in the appmixer connectors is already implemented at a different level (likely in lib.apiRequest), so additional try-catch blocks aren't needed in individual component files.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#461
File: src/appmixer/microsoft/sharepoint/FindFilesOrFolders/FindFilesOrFolders.js:0-0
Timestamp: 2025-04-22T10:39:55.176Z
Learning: The `makeRequest()` function imported from '../common' in the Microsoft SharePoint connector components already incorporates error handling through the `commons.formatError()` wrapper, making additional try-catch blocks around its calls redundant.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#461
File: src/appmixer/microsoft/sharepoint/FindFilesOrFolders/FindFilesOrFolders.js:0-0
Timestamp: 2025-04-22T10:39:55.176Z
Learning: The `makeRequest()` function imported from '../common' in the Microsoft SharePoint connector components already has built-in error handling, making additional try-catch blocks around its calls unnecessary.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#518
File: src/appmixer/canva/core/CreateDesign/CreateDesign.js:27-38
Timestamp: 2025-06-05T13:14:13.495Z
Learning: In the appmixer codebase for Canva connectors, try-catch blocks are not needed for HTTP requests in component files - the framework handles errors appropriately without explicit error handling.
src/appmixer/ai/mcptools/plugin.js (10)
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is widely used across the appmixer-connectors codebase, including in commons components of multiple connectors. While some files are being updated to use context.httpRequest instead, the dependency needs to remain in package.json files for components that still use it.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/** : Use `src/appmixer` for source code of connectors.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/*/{service.json,auth.js,bundle.json} : Connectors consist of service.json, auth.js, and bundle.json files, located in the `src/appmixer` folder.
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is still used in commons components of the Jira connector, and should be kept in package.json even though it was removed from auth.js.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#494
File: src/appmixer/ai/voyageai/CreateTextEmbeddingsFromFile/CreateTextEmbeddingsFromFile.js:36-40
Timestamp: 2025-04-30T12:27:08.628Z
Learning: In the appmixer-connectors codebase, `context.httpRequest` is an axios instance, so error handling should follow axios patterns (using try/catch blocks) rather than checking for `response.ok` which is a Fetch API pattern.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#460
File: src/appmixer/kafka/connections.js:3-7
Timestamp: 2025-04-04T08:08:44.744Z
Learning: In the appmixer-connectors project, dependencies for specific modules like Kafka should be added to their module-specific package.json files (e.g., src/appmixer/kafka/package.json) rather than the root package.json file.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#460
File: src/appmixer/kafka/connections.js:3-7
Timestamp: 2025-04-04T08:08:44.744Z
Learning: In the appmixer-connectors project, dependencies for specific modules like Kafka should be added to their module-specific package.json files (e.g., src/appmixer/kafka/package.json) rather than the root package.json file.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.822Z
Learning: Applies to src/appmixer/*/auth.js : The `auth.js` file for a connector must export an object with a `type` property set to either 'apiKey', 'pwd', or 'oauth2', and provide the required authentication definition structure.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to test/utils.js : Use `test/utils.js` for Appmixer stub.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#461
File: src/appmixer/microsoft/sharepoint/FindFilesOrFolders/FindFilesOrFolders.js:0-0
Timestamp: 2025-04-22T10:39:55.176Z
Learning: The `makeRequest()` function imported from '../common' in the Microsoft SharePoint connector components already has built-in error handling, making additional try-catch blocks around its calls unnecessary.
🪛 Gitleaks (8.26.0)
src/appmixer/ai/mcptools/routes.js
1-2: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 Biome (1.9.4)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 139-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/appmixer/ai/mcptools/lib.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/ai/mcptools/plugin.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (5)
src/appmixer/ai/mcptools/lib.js (1)
1-1: Static analysis warning about "use strict" can be ignored.The coding guidelines for
src/appmixer/**/*.jsfiles specifically require the "use strict" directive, so this static analysis warning should be disregarded.src/appmixer/ai/mcptools/plugin.js (1)
1-1: Static analysis warning about "use strict" can be ignored.The coding guidelines for
src/appmixer/**/*.jsfiles specifically require the "use strict" directive, so this static analysis warning should be disregarded.src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (1)
1-1: Static analysis warning about "use strict" can be ignored.The coding guidelines for
src/appmixer/**/*.jsfiles specifically require the "use strict" directive, so this static analysis warning should be disregarded.src/appmixer/ai/mcptools/routes.js (2)
1-2: Static analysis warning about auth header is a false positive.The static analysis tool flagged the authorization token in the curl comment, but this is just documentation showing API usage examples and doesn't represent a security risk.
19-163: Well-implemented routes and SSE functionality.The route handlers and SSE implementation are well-structured with proper authentication, error handling, and connection lifecycle management. The heartbeat mechanism and cleanup logic for SSE connections are particularly well done.
| @@ -0,0 +1,163 @@ | |||
| // curl -XGET \ | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add the "use strict" directive.
Per coding guidelines, the "use strict" directive is required for files matching src/appmixer/**/*.js.
+`use strict`;
+
// curl -XGET \
// -H "Authorization: Bearer ACCESS_TOKEN" \
// "https://APPMIXER_TENANT_API_URL/plugins/appmixer/ai/mcptools/gateways"Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Gitleaks (8.26.0)
1-2: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🤖 Prompt for AI Agents
In src/appmixer/ai/mcptools/routes.js at line 1, add the "use strict" directive
as the very first statement in the file to comply with coding guidelines for
files under src/appmixer/**/*.js.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (5)
113-122: Simplify the MCP server check.isMCPserver: function(context, componentId) { // Check if the component is an MCP server. const component = context.flowDescriptor[componentId]; if (!component) { return false; } - const category = component.type.split('.').slice(0, 2).join('.'); - const type = component.type.split('.').at(-1); - return category === 'appmixer.mcpservers' && type === 'MCPServer'; + return component.type === 'appmixer.mcpservers.MCPServer'; },
20-25: UsehttpRequestinstead ofcallAppmixerfor compatibility.To ensure compatibility with engine versions earlier than 6.1, use
context.httpRequestinstead ofcontext.callAppmixer.- return context.callAppmixer({ - endPoint: '/plugins/appmixer/ai/mcptools/gateways', - method: 'POST', - body: {} - }); + return context.httpRequest({ + url: `${process.env.APPMIXER_API_URL}/plugins/appmixer/ai/mcptools/gateways`, + method: 'POST', + data: {} + });
209-209: Fix potential issue with tool name length calculation.The tool name length limit calculation may not account for the actual componentId length correctly, potentially causing names to exceed the 64-character limit or result in invalid slice values.
- toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, 64 - componentId.length - 1); + const maxNameLength = Math.max(1, 64 - componentId.length - 1); + toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, maxNameLength);
240-252: Address potential race condition in polling loop.The polling loop could potentially miss results if the state is set and unset between checks, and there's no protection against multiple concurrent polls for the same tool call.
Add a flag to prevent concurrent polling:
+ const pollKey = `polling:${toolCall.id}`; + if (await context.flow.stateGet(pollKey)) { + return 'Error: Tool call already being processed.'; + } + await context.flow.stateSet(pollKey, true); + const pollStart = Date.now(); const pollTimeout = context.config.TOOLS_OUTPUT_POLL_TIMEOUT || TOOLS_OUTPUT_POLL_TIMEOUT; const pollInterval = context.config.TOOLS_OUTPUT_POLL_INTERVAL || TOOLS_OUTPUT_POLL_INTERVAL; while ((Date.now() - pollStart) < pollTimeout) { const result = await context.flow.stateGet(toolCall.id); if (result) { await context.flow.stateUnset(toolCall.id); + await context.flow.stateUnset(pollKey); return result.output; } // Sleep. await new Promise((resolve) => setTimeout(resolve, pollInterval)); } + await context.flow.stateUnset(pollKey); return 'Error: Tool timed out.';
36-40: UsehttpRequestfor consistency and compatibility.For consistency with the start method and compatibility with earlier engine versions.
- return context.callAppmixer({ - endPoint: '/plugins/appmixer/ai/mcptools/gateways/' + context.componentId, - method: 'DELETE' - }); + return context.httpRequest({ + url: `${process.env.APPMIXER_API_URL}/plugins/appmixer/ai/mcptools/gateways/${context.componentId}`, + method: 'DELETE' + });
🧹 Nitpick comments (3)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (3)
1-1: Remove redundant "use strict" directive.The "use strict" directive is automatically applied in JavaScript modules and is therefore redundant.
-'use strict'; -
66-66: Use optional chaining for cleaner code.- if (source[agentComponentId] && source[agentComponentId].includes(toolsPort)) { + if (source[agentComponentId]?.includes(toolsPort)) {
140-140: Use optional chaining for consistency.- if (source[agentComponentId] && source[agentComponentId].includes(mcpPort)) { + if (source[agentComponentId]?.includes(mcpPort)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`src/appmixer/**`: Use `src/appmixer` for source code of connectors.
src/appmixer/**: Usesrc/appmixerfor source code of connectors.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
`**/*.js`: Add one empty line after function definition. Use 4 spaces for indentation.
**/*.js: Add one empty line after function definition.
Use 4 spaces for indentation.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
`src/appmixer/**/*.js`: Not necessary to add try/catch blocks in their `receive`...
src/appmixer/**/*.js: Not necessary to add try/catch blocks in theirreceivefunction. Appmixer engine automatically handles any exceptions that originate in these async functions.The directive
"use strict";is desirable here
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/** : Use `src/appmixer` for source code of connectors.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.821Z
Learning: Applies to src/appmixer/*/{service.json,auth.js,bundle.json} : Connectors consist of service.json, auth.js, and bundle.json files, located in the `src/appmixer` folder.
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (10)
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#490
File: src/appmixer/ai/claude/AIAgent/AIAgent.js:70-72
Timestamp: 2025-05-06T10:06:04.289Z
Learning: In the AIAgent component, `toolUse.name` follows the format 'function_' + componentId where componentId is a UUID, so extracting it with `toolUse.name.split('_')[1]` is the correct approach.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#490
File: src/appmixer/ai/claude/AIAgent/AIAgent.js:70-72
Timestamp: 2025-05-06T10:06:04.289Z
Learning: In the AIAgent component, `toolUse.name` follows the format 'function_' + componentId where componentId is a UUID, so extracting it with `toolUse.name.split('_')[1]` is the correct approach.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/FindIssue/FindIssue.js:14-18
Timestamp: 2025-04-28T11:43:24.482Z
Learning: The FindIssue.js component in the GitHub connector should use encodeURIComponent when constructing search query strings to ensure special characters in repositoryId, title, and labels don't break the URL structure when making API requests.
Learnt from: CR
PR: clientIO/appmixer-connectors#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-30T09:09:52.822Z
Learning: Applies to src/appmixer/*/core/*/component.json : The `component.json` file's `name` property must match the pattern ^[\w]+\.[\w]+\.[\w]+\.[\w]+$.
Learnt from: jirihofman
PR: clientIO/appmixer-connectors#494
File: src/appmixer/ai/voyageai/CreateTextEmbeddingsFromFile/CreateTextEmbeddingsFromFile.js:36-40
Timestamp: 2025-04-30T12:27:08.628Z
Learning: In the appmixer-connectors codebase, `context.httpRequest` is an axios instance, so error handling should follow axios patterns (using try/catch blocks) rather than checking for `response.ok` which is a Fetch API pattern.
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is widely used across the appmixer-connectors codebase, including in commons components of multiple connectors. While some files are being updated to use context.httpRequest instead, the dependency needs to remain in package.json files for components that still use it.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#518
File: src/appmixer/canva/core/CreateDesign/CreateDesign.js:27-38
Timestamp: 2025-06-05T13:14:13.495Z
Learning: In the appmixer codebase for Canva connectors, try-catch blocks are not needed for HTTP requests in component files - the framework handles errors appropriately without explicit error handling.
Learnt from: vtalas
PR: clientIO/appmixer-connectors#424
File: src/appmixer/jira/package.json:6-8
Timestamp: 2025-03-17T09:26:28.766Z
Learning: "request-promise" dependency is still used in commons components of the Jira connector, and should be kept in package.json even though it was removed from auth.js.
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#444
File: src/appmixer/airtable/records/CreateOrUpdateRecord/CreateOrUpdateRecord.js:31-37
Timestamp: 2025-03-28T16:21:57.663Z
Learning: Error handling for HTTP requests in Airtable connector components is already handled at the component framework level, so additional try/catch blocks around context.httpRequest calls are not needed.
Learnt from: sayam-nasir
PR: clientIO/appmixer-connectors#448
File: src/appmixer/github/list/GetUser/GetUser.js:12-14
Timestamp: 2025-04-28T11:22:40.693Z
Learning: Error handling for GitHub API requests in the appmixer connectors is already implemented at a different level (likely in lib.apiRequest), so additional try-catch blocks aren't needed in individual component files.
🪛 Biome (1.9.4)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 140-140: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js (1)
269-283: Good error handling for MCP server calls.The try-catch block properly handles errors without triggering Appmixer component retries, returning error messages to the caller instead.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new ai.mcptools connector that enables integration with MCP (Model Context Protocol) servers, allowing AI assistants to discover and invoke tools from both local Appmixer flows and external MCP servers. The implementation includes a plugin architecture with HTTP API endpoints, real-time Server-Sent Events (SSE) for user notifications, and Redis-backed pub/sub for scalable event handling.
Key changes:
- New MCPGateway component for managing AI assistant tools within flows
- Plugin infrastructure with routes.js providing REST API and SSE endpoints
- Redis pub/sub integration for cross-pod event distribution
- Tool discovery and invocation supporting both Appmixer ToolStart components and external MCP servers
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/appmixer/ai/mcptools/routes.js | HTTP API endpoints for gateway management and SSE event streaming |
| src/appmixer/ai/mcptools/plugin.js | Plugin initialization with Redis client setup |
| src/appmixer/ai/mcptools/package.json | Dependencies for Redis, JWT, UUID handling |
| src/appmixer/ai/mcptools/module.json | Module metadata and category configuration |
| src/appmixer/ai/mcptools/lib.js | Redis connection utility with SSL/Sentinel support |
| src/appmixer/ai/mcptools/icon.png | Connector icon as binary PNG |
| src/appmixer/ai/mcptools/bundle.json | Bundle versioning and changelog |
| src/appmixer/ai/mcptools/MCPGateway/icon.svg | Component icon as SVG |
| src/appmixer/ai/mcptools/MCPGateway/component.json | Component manifest defining ports and metadata |
| src/appmixer/ai/mcptools/MCPGateway/MCPGateway.js | Component behavior implementing tool discovery, registration, and invocation logic |
| const pollStart = Date.now(); | ||
| const pollTimeout = context.config.TOOLS_OUTPUT_POLL_TIMEOUT || TOOLS_OUTPUT_POLL_TIMEOUT; | ||
| const pollInterval = context.config.TOOLS_OUTPUT_POLL_INTERVAL || TOOLS_OUTPUT_POLL_INTERVAL; | ||
| while ((Date.now() - pollStart) < pollTimeout) { | ||
| const result = await context.flow.stateGet(toolCall.id); | ||
| if (result) { | ||
| await context.flow.stateUnset(toolCall.id); | ||
| return result.output; | ||
| } | ||
| // Sleep. | ||
| await new Promise((resolve) => setTimeout(resolve, pollInterval)); | ||
| } |
There was a problem hiding this comment.
The polling mechanism could cause performance issues or resource exhaustion. The component polls with a 300ms interval for up to 2 minutes (400 iterations) while waiting for tool output. In a high-traffic scenario with many concurrent tool calls, this could create significant load.
Consider:
- Using Redis pub/sub for tool output notifications instead of polling
- Adding exponential backoff to reduce polling frequency over time
- Documenting the resource implications in the component description
| { | ||
| "name": "appmixer.ai.mcptools", | ||
| "label": "MCP Tools", | ||
| "category": "ai", | ||
| "categoryIndex": 0, | ||
| "index": 3, | ||
| "categoryLabel": "AI", | ||
| "description": "Generic tool components for building MCP servers.", | ||
| "icon": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMgAAADICAIAAAAiOjnJAAAPBUlEQVR4nOydf2wT5f/AW7pZGLOjE7K5DAfWIYMWM7rpWJTZkCxEh4hdcGKahYhkYRojGPQfUnCJMRpDlpD5hyEknZlWY2AL2UaiDubYLFkDzsE2CBlBrc5ldGm6VLjdnm++6Sf77DO758fdPb279v36kzz3ft73vhfPdffjfRkIIQMAKM0ytRMAUhMQC+ACiAVwAcQCuABiAVwAsQAugFgAF0AsgAsgFsAFEAvgAogFcAHEArgAYgFcALEALoBYABdALIALIBbABRAL4AKIBXABxAK4kKF2Anrlt99+6+vru3r16s2bN+/cuTM1NRWJRGZnZ1esWGGxWPLy8mw22+bNm8vLy7dt27Zy5Uq18002RnhLh4mhoaGvvvqqo6Pjxo0blJuYzebKykq32/3qq6+uXr2ac4KaAQEUiKL4zTffVFRUyCm12Wyur6+/du2a2nuTDEAsMu3t7Xa7Xan/ySaTae/evbdu3VJ7t/gCYuG4e/duTU2NUkotJCsry+v1CoKg9i7yAsRaEr/fb7VaeVg1zzPPPDM+Pq72jnIBxEqAKIoffPABV6XmWbVqVU9Pj9p7rDwg1mIEQairq0uOVXFMJlNbW5va+60wINb/cP/+/d27dyfTqlR1C8T6L2pZNe/W2bNn1a6BYsAF0v/w4MGDvXv3tre3q5hDdnb25cuXt2zZomIOSgFiGeRYZbVan3/++a1bt27YsCE3NzcjI0MQhD/++OP69es///xzIBAQBIEpYHFxcTAYfPjhh1kz0RxqL5nqI+EMaDQa3W53V1cX/kJUOBw+derUxo0bmYIfOnQoiXvPi3QXS4JVLpdraGiIfgpBEHw+X35+Pr21vb29PHc6GaS1WKxWmc3mU6dOSZtrenra7XZTTuRwOERRVHp3k0r6isVqldVq7evrkznpsWPHKKfz+/0K7ag6pKlYEqwKBoOKTH38+HGaGe12uyLTqUU6iqWiVXEaGhpo5tX1rZ60E0t1qxBCsVhs06ZNxKk9Ho+y8yaT9BJLC1bFGRgYMBqN+NktFkssFuMxexJII7G0Y1Wc2tpaYg4XLlzglwBX0uUtHdZr61ar9fvvv9+6dSu/lN5//33imJ6eHn4JcCUt3tLhbdXc3NyPP/7Y29sbDofXr1//8ssvP/7448StysrKHA7Hr7/+ihkzMDBAmYPmUHvJ5A7vM+CtW7fKy8sXRjCZTI2Njffv3ydu6/V6icnI23vVSHGxeFs1NjZWUFCQMFRdXR1x8wsXLhBTmpyclFcDdUhlsVS0Ks63336LjzA5OUnM6pdffpFdCRVIWbFUt8pgMOzatYsYZ/ny5fggOr1MmppiacEqg8FQWFhIDEWMc/78eXnFUIcUvNzA+2/AmzdvulyuUChEHDk3N6fIGD2SamJpxyqDwUDzkHEkEsEPWLFiBWVu2kLtJVNJNHIGnOe7777DB5yYmCAGuXr1quzCqEDqXCDV1FplMBh27979yiuv4McMDw8T42RmZg4ODobD4QcPHsQfNszNzS0oKKB/JFUd1DZbGbS2VlVVVUWjUWJY4gVSDBaLZfv27UePHu3s7NTgvepUEEunViGESktLJUm1GIvF8vrrr2vqwoTuxdKvVcFgUJJFODZt2nTmzBktNLHRt1j6tQoh5PF4JMlDprS09NKlS1KLqgw6FkvXVgUCAZPJJEkbWt54443p6Wmp1ZWLXsXStVWCIDgcDkm2sFFcXKzWrUZdiqVrqxBCjY2NkjyRgsViUaXXiP7E0rtVH3/8sSRDpKNKjySdiaV3qz755BNJbsgl+W7pSSywSg4mk+ncuXOSCi8F3YiV5lZlZ2c/9thjhYWFFotFThCmdiZy0IdY6WmV0+lsamq6dOnS1NTUwmhTU1M9PT1er/epp55ijblhw4ZIJMJSe4noQKx0s8poNHo8HsqlJRAI1NbWMl0SS07/La2LlW5WlZeXS/gmSiAQoL8wlpz+W5oWK92sOnz4sOTbfLFY7ODBg5QTlZaW8u6/pV2x0soqo9HY0tIiqU7/w4cffkg5I+/+WxoVK92sOn36tKQ6JYDSLYfDodSMCdGiWGCVTCjPifIbFGLQnFhglXwo+2/V19crPvU82hILrFKKvr4+Yv+tnJwcmgYT0tCQWGCVstD03/rhhx84za4VscAqxbl8+TIxk2PHjnGaXRNigVUY4p+j3rFjR35+fkFBQXV1Nf3zVU888QQ+merqavpMmFBfLLAKQzQafeGFF/4dp7a2lubnEfFznvn5+fTJMKGyWGAVhmg0WlVVtVS0t956ixihs7OTmBWn5+LVFAuswoC3Kh6Q+Cl8mve2b9y4QZ8VPaqJBVZhIFoV5+TJk/g4oihmZmbig3C6TKqOWGAVBkqrDAbDkSNHiNGIH+Ln9P60CmKBVRjorTIYDCdOnCAGfOSRR/BBUkQssAoDk1U0ToiiSHwGMBVOhWAVBlarKioqiDHv3r1LjKP7H+9gFQZWq4qKisbHx4lhz58/TwzF6RH4JIkFVmHgZBVC6OjRo/hQ+r5AClZh4GcVQqi4uBgfbefOnfSpMsFdLLAKA1er+vv7iQG9Xi99tkzwFQuswsDVKoRQwpuMi+DXRoujWGAVBt5W9fb2EmPm5OTw6/3HSyywCgNvq6LR6JNPPkkMe+DAAfqYrHARC6zCwNsqhND+/ftpIvf39zOFZUJ5sQRB2LNnD33hWK0aHx8HqzBQtvguKytjCsuK8mIx9S5ntWpqaqqoqIg+Pli1FLy//aSwWNPT01lZWZT7xmqVIAgul4v+wIBVS8F7uVJerO7ubsp9k/CN+BMnTtAfGLBqKUwmE9dfV3EUFuvMmTM0+ybBqmvXrtE36wGrMLz99ttMwaWhwoolwSpBEMrKyigLB1Zh2LhxI1NxJKOwWJFIJCcnB7NjEqxCCPl8PsrCgVUYLBYLp4dk/o3yfxVi2k1Ls0oQBJvNRlM4sAqDyWTq7Oxkii8HLtex9u3b9+8dy8vLk2AVQujs2bM0hausrASrliJ12nH7fD6n0xnvS5GXl3fo0KFQKCQt1I4dO4iFKywsnJiYoI8JVvGG79MNsVgsHA7LiRAKhWj+GOzq6qKPCVYlAfVfscfT0tJCrJ3b7aYPCFYlB62LVVNTQ6zdyMgIZTSwKmloWixRFFetWoUvH32/FLAqmWharJGREWIFfT4fTSiwKsloWqxz584Ri0jzxyBYlXw0LdZnn32GL6LNZiMG+fTTT+mPClilFJoW68iRI/g6vvjii/gIXV1dxB6v84BVCqJpsYj9yg8ePIiP4HQ6KY8KWKUsmharvr4eX83GxkbM5jSdC+KAVYqjabGILwU0NDRgNqd5BQqs4oSmxXrnnXfwNa2trcVsPjw8TDwqYBUnNC1WU1MTvqxbtmzBbC6KYn5+PmZzsIofmhbryy+/xFc2MzNzZmYGE+HkyZNglSpoWqwrV64Q69vd3Y2JIIpiXV1dwqMCVnFF02LNzMwQm/7u378fH0QQhObm5oXnRKfTyfQNGbBKApoWCyFUUVGBr3J2dva9e/eIcURRHB0dvXLlCuvzhmCVNLQuFk3R+X1pCKySjNbFCgaDxHJnZWXdvn1b8anBKjloXSyEEM3HQquqqpRt9QRWyUQHYmEuGSzk3XffVWpGsEo+OhDr3r17+Jdg51Gko2YkEgGr5KMDsZiOxOHDh+WcE0Oh0NNPPw1WyUcfYk1PT+Nvzixk+/btxO+tJaS9vZ1+FrAKjz7EYmrfEP878b333qN/hXVoaIipCyFYRUQ3YiGEdu3axXTss7KyPB5PR0fHUq/eh0KhL774wuVy0T9lClZRYvx/uXTC33//7XQ6f//9d9YNMzMzS0pKbDbbmjVrMjIyYrHYn3/+OTo6eufOHQlpFBUVXbx4cd26dfSbHD9+nL5rnMlkam1tfe211yTkpiHUNpuN/v7+5cuXq1guWKso0ZlYCCG/30/f2k9ZwCp69CcWQqitrS35boFVTOhSrPi6lcxzYnFxMVjFhF7FQggNDAwUFhby1Ok/VFdXszZjSnOr9C0WQmhiYoLYjkYOZrP5o48+EkWRKSuwSvdixWlra1uzZo3iVj377LPDw8OsyYBVcVJBrPjzCF6v12q1KqKU3W73+/2sCxVYtZAUEStONBptbm622+3SfDIajTU1NR0dHRKUAqsWkVJizRMMBpuamiorK81mM/EYW63WPXv2tLS0/PXXX5JnBKsWoadbOhL4559/RkdHr1+/HgqFJiYmZmZmYrHYypUrLRbLo48+un79+s2bN69bt27ZsmVyZknHOzZE1DZb98BalRAQSxZg1VKAWNIBqzCAWBIBq/CAWFIAq4iAWMyAVTSAWGyAVZSAWAyAVfSAWLSAVUyAWFSAVayAWGTAKgmAWAQoW5KAVYtI8ZvQMvnpp59cLpcoijSD0+XuMh0g1pLMzs7a7faxsTGawWDVImQ9LpLanD59GqySDKxYS1JSUjI6OkocBlYlBFasxIyMjIBVcgCxEnPx4kXiGLAKA4iVGOLnqMEqPCBWYiYnJ/ED3G43WIUBxErM3NwcfsDq1auTlYsuAbESk5ubix/g8/kGBgaSlY7+ALESY7PZ8AOi0ejOnTvBraUAsRLz3HPPEcdEIhFwayngAmli5ubm1q5dGwqFiCMtFkt3d/e2bduSkpdugBUrMcuWLXvzzTdpRsK6lRBYsZYkHA7bbLZwOEwzGNatRcCKtSRWq7W5uZlyMKxbiwCxcHg8ngMHDlAOBrcWAqdCArOzsx6P5+uvv6YcD+fEOLBiEcjIyGhtbU34KfyEwLoVB8QiA25JAMSiAtxiBX5jMSDh91ZfX5/D4eCclxYBsdhgdcvpdA4ODnJOSovAqZAN1nNiMBgEsQAqWN0KBAKcM9IiIJYUmNwSBIF/RpoDxJIIvVslJSVJyUhbwI93WRB/yxcUFNy+fVvdr8KqAqxYssCvW0aj8fPPP09Dq0AsBYi71dDQsOjfs7OzW1tbX3rpJZXyUhk4FSrG4OCg3+8fGxt76KGHysvL9+3bt3btWrWTUg0QC+ACnAoBLoBYABdALIALIBbABRAL4AKIBXABxAK4AGIBXACxAC6AWAAXQCyACyAWwAUQC+ACiAVwAcQCuABiAVz4vwAAAP//b8cbMGXTzMEAAAAASUVORK5CYII=" | ||
| } |
There was a problem hiding this comment.
The connector is missing a required service.json file. According to Appmixer connector development guidelines, every connector must have a service.json file that describes the service metadata including name, label, description, category, version, and icon.
Expected structure:
{
"name": "appmixer.ai.mcptools",
"label": "MCP Tools",
"category": "applications",
"description": "Description of what this connector does",
"version": "1.0.0",
"icon": "https://example.com/icon.svg"
}This file should be created at src/appmixer/ai/mcptools/service.json alongside the existing module.json and bundle.json files.
| if (component.type !== 'appmixer.ai.agenttools.ToolStart') { | ||
| error = `Component ${componentId} is not of type 'ToolStart' but ${component.type}. | ||
| Every tool chain connected to the '${toolsPort}' port of the AI Agent | ||
| must start with 'ToolStart' and end with 'ToolOutput'. | ||
| This is where you describe what the tool does and what parameters should the AI model provide to it.`; | ||
| } |
There was a problem hiding this comment.
The component references appmixer.ai.agenttools.ToolStart which appears to be from a different connector (ai.agenttools) but this is the ai.mcptools connector. This cross-connector dependency should be clearly documented, or the error message should be updated to reflect the correct component type for this connector's architecture.
If tools are meant to use components from ai.agenttools, this dependency relationship needs to be documented in the connector's README or component description.
| // Make sure listeners are removed so that the ones registered in routes.js | ||
| // will not introduce duplicate message handlers. | ||
| // TODO: only listenrs that are registered by this plugin should be removed. |
There was a problem hiding this comment.
Incomplete TODO comment without implementation. The comment states "only listeners that are registered by this plugin should be removed" but provides no implementation. Either implement the selective listener removal or remove this incomplete TODO comment.
If this is a known limitation, it should be documented more clearly with a proper explanation of the consequences.
| // Make sure listeners are removed so that the ones registered in routes.js | |
| // will not introduce duplicate message handlers. | |
| // TODO: only listenrs that are registered by this plugin should be removed. | |
| // Reuse existing Redis Subscriber client but clear all previously | |
| // registered listeners to avoid duplicate message handlers from this | |
| // plugin when it is re-initialized in the same process. Note that this | |
| // will also remove any listeners registered elsewhere on this client. | |
| process.CONNECTOR_STREAM_SUB_CLIENT.removeAllListeners(); |
| let componentId = req.data.function.name.split('_')[0]; | ||
| const toolName = req.data.function.name.split('_').slice(1).join('_'); | ||
| const args = typeof req.data.function.arguments === 'string' ? JSON.parse(req.data.function.arguments) : req.data.function.arguments; |
There was a problem hiding this comment.
Missing input validation. The webhook payload is accessed without validating that the expected structure exists. This could cause runtime errors if malformed requests are received.
Add validation before accessing nested properties:
if (!req.data?.function?.name) {
return context.response('Invalid request: missing function name', 400);
}Similar validation should be added for req.data.function.arguments.
| let componentId = req.data.function.name.split('_')[0]; | |
| const toolName = req.data.function.name.split('_').slice(1).join('_'); | |
| const args = typeof req.data.function.arguments === 'string' ? JSON.parse(req.data.function.arguments) : req.data.function.arguments; | |
| // Validate expected structure before accessing nested properties. | |
| if (!req?.data?.function?.name) { | |
| return context.response('Invalid request: missing function name', 400); | |
| } | |
| if (typeof req.data.function.arguments === 'undefined') { | |
| return context.response('Invalid request: missing function arguments', 400); | |
| } | |
| let componentId = req.data.function.name.split('_')[0]; | |
| const toolName = req.data.function.name.split('_').slice(1).join('_'); | |
| let args = req.data.function.arguments; | |
| if (typeof args === 'string') { | |
| try { | |
| args = JSON.parse(args); | |
| } catch (e) { | |
| return context.response('Invalid request: malformed function arguments JSON', 400); | |
| } | |
| } |
| let toolName = (component.label || component.type.split('.').pop()); | ||
| toolName = toolName.replace(/[^a-zA-Z0-9_]/g, '_').slice(0, 64 - componentId.length - 1); | ||
| const toolDefinition = { | ||
| type: 'function', | ||
| function: { | ||
| name: componentId + '_' + toolName, |
There was a problem hiding this comment.
The tool name sanitization logic has potential issues:
- The slice operation
slice(0, 64 - componentId.length - 1)could result in negative or very small values if componentId is long (UUIDs are 36 chars), leaving only ~27 chars for the tool name - Multiple tools could end up with the same name after truncation and sanitization
- No validation that the resulting name is non-empty
Consider:
- Using a hash or short ID instead of the full componentId
- Validating that the final tool name is unique and non-empty
- Documenting the 64-character limit and why it exists
| module.exports = (context) => { | ||
|
|
||
| // assert(Redis clients have been connected in plugin.js). | ||
| process.CONNECTOR_STREAM_SUB_CLIENT.psubscribe('stream:mcp:events:*'); |
There was a problem hiding this comment.
The psubscribe call is made synchronously without awaiting or error handling. If the Redis subscription fails, it will fail silently and SSE events won't be delivered.
Add proper error handling:
try {
await process.CONNECTOR_STREAM_SUB_CLIENT.psubscribe('stream:mcp:events:*');
await context.log('info', '[AI.MCPTOOLS] Subscribed to Redis channel pattern');
} catch (err) {
await context.log('error', `[AI.MCPTOOLS] Failed to subscribe to Redis: ${err.message}`);
throw err;
}| module.exports = (context) => { | |
| // assert(Redis clients have been connected in plugin.js). | |
| process.CONNECTOR_STREAM_SUB_CLIENT.psubscribe('stream:mcp:events:*'); | |
| module.exports = async (context) => { | |
| // assert(Redis clients have been connected in plugin.js). | |
| try { | |
| await process.CONNECTOR_STREAM_SUB_CLIENT.psubscribe('stream:mcp:events:*'); | |
| await context.log('info', '[AI.MCPTOOLS] Subscribed to Redis channel pattern'); | |
| } catch (err) { | |
| await context.log('error', `[AI.MCPTOOLS] Failed to subscribe to Redis: ${err.message}`); | |
| throw err; | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
3 participants
Summary by CodeRabbit
New Features
Chores