From 9bc2b163f3366416926a549c99ac70f8b5c48b92 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Tue, 23 Dec 2025 14:24:10 -0500 Subject: [PATCH 1/3] feat(tools-user): user facing tools-as-plugins --- eslint.config.js | 20 +- jest.config.ts | 20 +- package-lock.json | 11 + package.json | 9 +- .../options.defaults.test.ts.snap | 2 + .../__snapshots__/server.tools.test.ts.snap | 549 ++++++++++++ .../server.toolsUser.test.ts.snap | 484 +++++++++++ src/__tests__/server.helpers.test.ts | 89 +- src/__tests__/server.tools.test.ts | 779 ++++++++++++++++++ src/__tests__/server.toolsUser.test.ts | 642 +++++++++++++++ src/options.defaults.ts | 25 +- src/server.caching.ts | 8 +- src/server.helpers.ts | 10 + src/server.tools.ts | 655 +++++++++++++++ src/server.toolsUser.ts | 759 +++++++++++++++++ src/tool.componentSchemas.ts | 5 + src/tool.fetchDocs.ts | 5 + src/tool.patternFlyDocs.ts | 5 + 18 files changed, 4068 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/__snapshots__/server.tools.test.ts.snap create mode 100644 src/__tests__/__snapshots__/server.toolsUser.test.ts.snap create mode 100644 src/__tests__/server.tools.test.ts create mode 100644 src/__tests__/server.toolsUser.test.ts create mode 100644 src/server.tools.ts create mode 100644 src/server.toolsUser.ts diff --git a/eslint.config.js b/eslint.config.js index c94ae35..4bf7889 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -81,7 +81,25 @@ export default [ }], 'n/no-process-exit': 0, // Disallow console.log/info in runtime to protect STDIO; allow warn/error - 'no-console': ['error', { allow: ['warn', 'error'] }] + 'no-console': ['error', { allow: ['warn', 'error'] }], + // Custom syntax rules + 'no-restricted-syntax': [ + 'error', + { + // Disallow rest parameters in a property named `keyHash` + selector: + "Property[key.name='keyHash'] > :matches(FunctionExpression, ArrowFunctionExpression) > RestElement", + message: + 'keyHash must accept a single array parameter (args). Do not use rest params (...args).' + }, + { + // Also catch when `keyHash` lives in a CallExpression options object (e.g., memo(fn, { keyHash() {} })) + selector: + "CallExpression > ObjectExpression > Property[key.name='keyHash'] > :matches(FunctionExpression, ArrowFunctionExpression) > RestElement", + message: + 'keyHash must accept a single array parameter (args). Do not use rest params (...args).' + } + ] } }, { diff --git a/jest.config.ts b/jest.config.ts index 5c740a5..dd8b222 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -25,7 +25,25 @@ export default { roots: ['src'], testMatch: ['/src/**/*.test.ts'], setupFilesAfterEnv: ['/jest.setupTests.ts'], - ...baseConfig + ...baseConfig, + transform: { + '^.+\\.(ts|tsx)$': [ + 'ts-jest', + { + ...tsConfig, + diagnostics: { + ignoreCodes: [1343] + }, + astTransformers: { + before: [ + { + path: 'ts-jest-mock-import-meta' + } + ] + } + } + ] + } }, { displayName: 'e2e', diff --git a/package-lock.json b/package-lock.json index 7930e8f..7776a93 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,6 +31,7 @@ "jest": "^30.2.0", "pkgroll": "^2.20.1", "ts-jest": "29.4.4", + "ts-jest-mock-import-meta": "^1.3.1", "ts-node": "^10.1.0", "tsx": "^4.21.0", "typescript": "^5.9.3", @@ -10765,6 +10766,16 @@ } } }, + "node_modules/ts-jest-mock-import-meta": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/ts-jest-mock-import-meta/-/ts-jest-mock-import-meta-1.3.1.tgz", + "integrity": "sha512-KGrp9Nh/SdyrQs5hZvtkp0CFPOgAh3DL57NZgFRbtlvMyEo7XuXLbeyylmxFZGGu30pL338h9KxwSxrNDndygw==", + "dev": true, + "license": "MIT", + "peerDependencies": { + "ts-jest": ">=20.0.0" + } + }, "node_modules/ts-jest/node_modules/semver": { "version": "7.7.3", "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.3.tgz", diff --git a/package.json b/package.json index fd6492d..efc07e6 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,14 @@ "description": "PatternFly documentation MCP server built with Node.js and TypeScript", "main": "dist/index.js", "type": "module", + "imports": { + "#toolsHost": "./dist/server.toolsHost.js" + }, "exports": { - ".": "./dist/index.js" + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + } }, "bin": { "patternfly-mcp": "dist/cli.js", @@ -64,6 +70,7 @@ "jest": "^30.2.0", "pkgroll": "^2.20.1", "ts-jest": "29.4.4", + "ts-jest-mock-import-meta": "^1.3.1", "ts-node": "^10.1.0", "tsx": "^4.21.0", "typescript": "^5.9.3", diff --git a/src/__tests__/__snapshots__/options.defaults.test.ts.snap b/src/__tests__/__snapshots__/options.defaults.test.ts.snap index 0ace401..b639fca 100644 --- a/src/__tests__/__snapshots__/options.defaults.test.ts.snap +++ b/src/__tests__/__snapshots__/options.defaults.test.ts.snap @@ -37,6 +37,7 @@ exports[`options defaults should return specific properties: defaults 1`] = ` "invokeTimeoutMs": 10000, "loadTimeoutMs": 5000, }, + "pluginIsolation": "none", "repoName": "patternfly-mcp", "resourceMemoOptions": { "default": { @@ -70,6 +71,7 @@ exports[`options defaults should return specific properties: defaults 1`] = ` "expire": 60000, }, }, + "toolModules": [], "urlRegex": /\\^\\(https\\?:\\)\\\\/\\\\//i, "version": "0.0.0", } diff --git a/src/__tests__/__snapshots__/server.tools.test.ts.snap b/src/__tests__/__snapshots__/server.tools.test.ts.snap new file mode 100644 index 0000000..76a1d6f --- /dev/null +++ b/src/__tests__/__snapshots__/server.tools.test.ts.snap @@ -0,0 +1,549 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`composeTools should attempt to setup creators, default package creators 1`] = ` +{ + "log": [], + "toolsCount": 3, +} +`; + +exports[`composeTools should attempt to setup creators, file package creators 1`] = ` +{ + "log": [], + "toolsCount": 5, +} +`; + +exports[`composeTools should attempt to setup creators, file package creators, Node.js 20 1`] = ` +{ + "log": [ + [ + "External tool plugins require Node >= 22; skipping file-based tools.", + ], + ], + "toolsCount": 3, +} +`; + +exports[`composeTools should attempt to setup creators, file package creators, Node.js 24 1`] = ` +{ + "log": [], + "toolsCount": 5, +} +`; + +exports[`composeTools should attempt to setup creators, file package creators, Node.js undefined 1`] = ` +{ + "log": [ + [ + "External tool plugins require Node >= 22; skipping file-based tools.", + ], + ], + "toolsCount": 3, +} +`; + +exports[`composeTools should attempt to setup creators, file package duplicate creators 1`] = ` +{ + "log": [ + [ + "Skipping plugin tool "@patternfly/tools" – name already used by built-in/inline tool.", + ], + ], + "toolsCount": 5, +} +`; + +exports[`composeTools should attempt to setup creators, inline and file package creators 1`] = ` +{ + "log": [], + "toolsCount": 7, +} +`; + +exports[`composeTools should attempt to setup creators, inline and file package creators duplicate builtin creators 1`] = ` +{ + "log": [ + [ + "Skipping inline tool "loremipsum" because a tool with the same name is already provided (built-in or earlier).", + ], + [ + "Skipping plugin tool "dolorSitAmet" – name already used by built-in/inline tool.", + ], + ], + "toolsCount": 3, +} +`; + +exports[`composeTools should attempt to setup creators, inline and file package creators, duplicates 1`] = ` +{ + "log": [ + [ + "Skipping plugin tool "@patternfly/tools" – name already used by built-in/inline tool.", + ], + [ + "Skipping plugin tool "DOLOR " – name already used by built-in/inline tool.", + ], + ], + "toolsCount": 6, +} +`; + +exports[`composeTools should attempt to setup creators, inline and file package creators, duplicates, Node.js 20 1`] = ` +{ + "log": [ + [ + "External tool plugins require Node >= 22; skipping file-based tools.", + ], + ], + "toolsCount": 5, +} +`; + +exports[`composeTools should attempt to setup creators, inline creators 1`] = ` +{ + "log": [], + "toolsCount": 5, +} +`; + +exports[`composeTools should attempt to setup creators, inline duplicate creators 1`] = ` +{ + "log": [ + [ + "Skipping inline tool "dolor" because a tool with the same name is already provided (built-in or earlier).", + ], + ], + "toolsCount": 5, +} +`; + +exports[`composeTools should attempt to setup creators, invalid creator 1`] = ` +{ + "log": [ + [ + "createMcpTool: invalid configuration used at index 0: Unsupported type object", + ], + ], + "toolsCount": 3, +} +`; + +exports[`composeTools should return default creators on tools host error 1`] = ` +{ + "log": [ + [ + "Failed to start Tools Host; skipping externals and continuing with built-ins/inline. undefined", + ], + ], + "toolsCount": 3, +} +`; + +exports[`debugChild should attempt to highlight specific messages, access denied 1`] = ` +{ + "debug": [], + "warn": [ + [ + "Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet +Tools Host denied fs read. In strict mode, add the resource's directory to --allow-fs-read. +Optionally, you can disable strict mode entirely with pluginIsolation: 'none'.", + ], + ], +} +`; + +exports[`debugChild should attempt to highlight specific messages, access denied, alt messaging 1`] = ` +{ + "debug": [], + "warn": [ + [ + "Error [ERR_ACCESS_DENIED]: fs.readFileSync access is denied by permission model: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet +Tools Host denied fs read. In strict mode, add the resource's directory to --allow-fs-read. +Optionally, you can disable strict mode entirely with pluginIsolation: 'none'.", + ], + [ + "Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet +Tools Host denied fs read. In strict mode, add the resource's directory to --allow-fs-read. +Optionally, you can disable strict mode entirely with pluginIsolation: 'none'.", + ], + ], +} +`; + +exports[`debugChild should attempt to highlight specific messages, access denied, multiple lines 1`] = ` +{ + "debug": [ + [ + "[tools-host pid=123 sid=1234567890] Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet", + ], + ], + "warn": [ + [ + "Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet +Tools Host denied fs read. In strict mode, add the resource's directory to --allow-fs-read. +Optionally, you can disable strict mode entirely with pluginIsolation: 'none'.", + ], + ], +} +`; + +exports[`debugChild should attempt to highlight specific messages, default 1`] = ` +{ + "debug": [ + [ + "[tools-host pid=123 sid=1234567890] lorem ipsum dolor sit amet", + ], + ], + "warn": [], +} +`; + +exports[`debugChild should attempt to highlight specific messages, empty string 1`] = ` +{ + "debug": [], + "warn": [], +} +`; + +exports[`debugChild should attempt to highlight specific messages, generic multiline error 1`] = ` +{ + "debug": [ + [ + "[tools-host pid=123 sid=1234567890] Lorem ipsum", + ], + [ + "[tools-host pid=123 sid=1234567890] dolor sit", + ], + [ + "[tools-host pid=123 sid=1234567890] amet", + ], + ], + "warn": [], +} +`; + +exports[`debugChild should attempt to highlight specific messages, generic multiline error with spaces 1`] = ` +{ + "debug": [ + [ + "[tools-host pid=123 sid=1234567890] Lorem ipsum", + ], + [ + "[tools-host pid=123 sid=1234567890] dolor sit", + ], + [ + "[tools-host pid=123 sid=1234567890] amet", + ], + ], + "warn": [], +} +`; + +exports[`debugChild should attempt to highlight specific messages, module not found 1`] = ` +{ + "debug": [], + "warn": [ + [ + "Tools Host import error. Ensure external tools are ESM (no raw .ts) and resolvable. +For local files, prefer a file:// URL.", + ], + ], +} +`; + +exports[`debugChild should attempt to highlight specific messages, module not found, multiple lines 1`] = ` +{ + "debug": [ + [ + "[tools-host pid=123 sid=1234567890] Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lorem/ipsum/dolor/sit/amet' imported from /test/path", + ], + ], + "warn": [ + [ + "Tools Host import error. Ensure external tools are ESM (no raw .ts) and resolvable. +For local files, prefer a file:// URL.", + ], + ], +} +`; + +exports[`getFilePackageToolModules, should return filtered tool modules 1`] = ` +[ + "@scope/pkg", + "file:///test/module.js", + "http://example.com/module.js", + "https://example.com/module.js", +] +`; + +exports[`logWarningsErrors should log warnings and errors, with both warnings and errors 1`] = ` +[ + [ + "Tools load warnings (1) + - Warning 1", + ], + [ + "Tools load errors (1) + - Error 1", + ], +] +`; + +exports[`logWarningsErrors should log warnings and errors, with empty arrays 1`] = `[]`; + +exports[`logWarningsErrors should log warnings and errors, with errors only 1`] = ` +[ + [ + "Tools load errors (2) + - Error 1 + - Error 2", + ], +] +`; + +exports[`logWarningsErrors should log warnings and errors, with single error 1`] = ` +[ + [ + "Tools load errors (1) + - Single error", + ], +] +`; + +exports[`logWarningsErrors should log warnings and errors, with single warning 1`] = ` +[ + [ + "Tools load warnings (1) + - Single warning", + ], +] +`; + +exports[`logWarningsErrors should log warnings and errors, with undefined warnings and errors 1`] = `[]`; + +exports[`logWarningsErrors should log warnings and errors, with warnings only 1`] = ` +[ + [ + "Tools load warnings (2) + - Warning 1 + - Warning 2", + ], +] +`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with error: handler 1`] = `[Error: Error message]`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with error: send 1`] = ` +[ + [ + undefined, + { + "args": { + "loremIpsum": 7, + }, + "id": "id-1", + "t": "invoke", + "toolId": "loremIpsum", + }, + ], +] +`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with full error: handler 1`] = `[Error: Error message]`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with full error: send 1`] = ` +[ + [ + undefined, + { + "args": { + "loremIpsum": 7, + }, + "id": "id-1", + "t": "invoke", + "toolId": "loremIpsum", + }, + ], +] +`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false: handler 1`] = `[Error: Tool invocation failed]`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false: send 1`] = ` +[ + [ + undefined, + { + "args": { + "loremIpsum": 7, + }, + "id": "id-1", + "t": "invoke", + "toolId": "loremIpsum", + }, + ], +] +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, basic 1`] = ` +{ + "debug": [], + "output": [ + [ + "Lorem Ipsum", + { + "description": "Lorem ipsum dolor sit amet", + "inputSchema": "isZod = true", + }, + [Function], + ], + ], +} +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, no tools 1`] = ` +{ + "debug": [], + "output": [], +} +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, null JSON input schema 1`] = ` +{ + "debug": [ + [ + "Tool "Lorem Ipsum" from unknown source failed strict JSON to Zod reconstruction.", + "Using fallback best-effort schema. Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.", + "[ZOD_SCHEMA: defined: true]", + ], + ], + "output": [ + [ + "Lorem Ipsum", + { + "description": "Lorem ipsum dolor sit amet", + "inputSchema": "isZod = true", + }, + [Function], + ], + ], +} +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, undefined JSON input schema 1`] = ` +{ + "debug": [ + [ + "Tool "Lorem Ipsum" from unknown source failed strict JSON to Zod reconstruction.", + "Using fallback best-effort schema. Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.", + "[ZOD_SCHEMA: defined: true]", + ], + ], + "output": [ + [ + "Lorem Ipsum", + { + "description": "Lorem ipsum dolor sit amet", + "inputSchema": "isZod = true", + }, + [Function], + ], + ], +} +`; + +exports[`sendToolsHostShutdown should attempt force shutdown of child and fail NaN`] = ` +[ + [ + "Failed to send shutdown signal to Tools Host child process: Error: Mock send failure", + ], + [ + "Failed to force-kill Tools Host child process: Error: Mock failed to kill child process", + ], + [ + "Failed to close Tools Host stderr reader: Error: Mock close failure 1", + ], + [ + "Slow shutdown response. Primary fallback force-killing Tools Host child process.", + ], +] +`; + +exports[`spawnToolsHost attempt to spawn the Tools Host, with no pluginIsolation, node 24: spawn 1`] = ` +{ + "spawn": [ + [ + "/mock/path/to/toolsHost.js", + ], + { + "stdio": [ + "ignore", + "pipe", + "pipe", + "ipc", + ], + }, + ], +} +`; + +exports[`spawnToolsHost attempt to spawn the Tools Host, with strict pluginIsolation, node 22: spawn 1`] = ` +{ + "spawn": [ + [ + "--experimental-permission", + "--allow-fs-read=/", + "--allow-fs-read=/mock/path/to", + "/mock/path/to/toolsHost.js", + ], + { + "stdio": [ + "ignore", + "pipe", + "pipe", + "ipc", + ], + }, + ], +} +`; + +exports[`spawnToolsHost attempt to spawn the Tools Host, with strict pluginIsolation, node 24: spawn 1`] = ` +{ + "spawn": [ + [ + "--permission", + "--allow-fs-read=/", + "--allow-fs-read=/mock/path/to", + "/mock/path/to/toolsHost.js", + ], + { + "stdio": [ + "ignore", + "pipe", + "pipe", + "ipc", + ], + }, + ], +} +`; + +exports[`spawnToolsHost attempt to spawn the Tools Host, with undefined pluginIsolation, node 22: spawn 1`] = ` +{ + "spawn": [ + [ + "/mock/path/to/toolsHost.js", + ], + { + "stdio": [ + "ignore", + "pipe", + "pipe", + "ipc", + ], + }, + ], +} +`; diff --git a/src/__tests__/__snapshots__/server.toolsUser.test.ts.snap b/src/__tests__/__snapshots__/server.toolsUser.test.ts.snap new file mode 100644 index 0000000..730949c --- /dev/null +++ b/src/__tests__/__snapshots__/server.toolsUser.test.ts.snap @@ -0,0 +1,484 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`createMcpTool should normalize configs, a creator 1`] = ` +[ + [Function], +] +`; + +exports[`createMcpTool should normalize configs, an object 1`] = ` +[ + [Function], +] +`; + +exports[`createMcpTool should normalize configs, array of creators 1`] = ` +[ + [Function], + [Function], +] +`; + +exports[`createMcpTool should normalize configs, mix of package, object, tuple, creator 1`] = ` +[ + "@scope/pkg", + [Function], + [Function], + [Function], +] +`; + +exports[`createMcpTool should normalize configs, nested createMcpTool calls 1`] = ` +[ + "@scope/pkg1", + "@scope/pkg2", + "@scope/pkg3", + [Function], + [Function], + "@scope/pkg4", + "@scope/pkg5", +] +`; + +exports[`createMcpTool should normalize configs, single tuple 1`] = ` +[ + [Function], +] +`; + +exports[`normalizeFilePackage handles absolute file path 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": true, + "isUrlLike": false, + "normalizedUrl": "/package.json", + "original": "/package.json", + "type": "file", + "value": "/package.json", +} +`; + +exports[`normalizeFilePackage handles file URL 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": true, + "isUrlLike": true, + "normalizedUrl": "/package.json", + "original": "/package.json", + "type": "file", + "value": "/package.json", +} +`; + +exports[`normalizeFilePackage handles http URL (not file) 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": false, + "isUrlLike": true, + "normalizedUrl": "/module.mjs", + "original": "/module.mjs", + "type": "package", + "value": "/module.mjs", +} +`; + +exports[`normalizeFilePackage handles invalid file URLs, encoding 1`] = ` +{ + "error": "true", + "fsReadDir": "/", + "isFilePath": false, + "isFileUrl": true, + "isUrlLike": true, + "normalizedUrl": "/%E0%A4%A", + "original": "/%E0%A4%A", + "type": "invalid", + "value": "/%E0%A4%A", +} +`; + +exports[`normalizeFilePackage handles invalid file URLs, hostname 1`] = `undefined`; + +exports[`normalizeFilePackage handles null 1`] = `undefined`; + +exports[`normalizeFilePackage handles number 1`] = `undefined`; + +exports[`normalizeFilePackage handles package name string 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": false, + "isFileUrl": false, + "isUrlLike": false, + "normalizedUrl": "/pkg", + "original": "/pkg", + "type": "package", + "value": "/pkg", +} +`; + +exports[`normalizeFilePackage handles relative file path 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": true, + "isUrlLike": false, + "normalizedUrl": "/package.json", + "original": "/package.json", + "type": "file", + "value": "/package.json", +} +`; + +exports[`normalizeFilePackage handles undefined 1`] = `undefined`; + +exports[`normalizeFilePath handles absolute file path 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": true, + "isUrlLike": false, + "normalizedUrl": "/package.json", + "original": "/package.json", + "type": "file", + "value": "/package.json", +} +`; + +exports[`normalizeFilePath handles file URL 1`] = `undefined`; + +exports[`normalizeFilePath handles package name string 1`] = `undefined`; + +exports[`normalizeFilePath handles relative file path 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": true, + "isUrlLike": false, + "normalizedUrl": "/package.json", + "original": "/package.json", + "type": "file", + "value": "/package.json", +} +`; + +exports[`normalizeFileUrl handles absolute file path 1`] = `undefined`; + +exports[`normalizeFileUrl handles file URL 1`] = ` +{ + "fsReadDir": "/", + "isFilePath": true, + "isFileUrl": true, + "isUrlLike": true, + "normalizedUrl": "/package.json", + "original": "/package.json", + "type": "file", + "value": "/package.json", +} +`; + +exports[`normalizeFileUrl handles http URL (not file) 1`] = `undefined`; + +exports[`normalizeFileUrl handles invalid file URLs, encoding 1`] = ` +{ + "error": "true", + "fsReadDir": "/", + "isFilePath": false, + "isFileUrl": true, + "isUrlLike": true, + "normalizedUrl": "/%E0%A4%A", + "original": "/%E0%A4%A", + "type": "invalid", + "value": "/%E0%A4%A", +} +`; + +exports[`normalizeFileUrl handles invalid file URLs, hostname 1`] = `undefined`; + +exports[`normalizeFileUrl handles package name string 1`] = `undefined`; + +exports[`normalizeFileUrl handles relative file path 1`] = `undefined`; + +exports[`normalizeFunction should normalize the config, basic 1`] = ` +[ + "loremIpsum", + { + "description": "lorem ipsum", + "inputSchema": { + "properties": {}, + "type": "object", + }, + }, + [Function], +] +`; + +exports[`normalizeFunction should normalize the config, missing handler 1`] = ` +[ + "dolorSit", + { + "description": "x", + }, +] +`; + +exports[`normalizeFunction should normalize the config, missing schema 1`] = ` +[ + "dolorSit", + { + "description": "x", + }, + [Function], +] +`; + +exports[`normalizeFunction should normalize the config, null 1`] = `null`; + +exports[`normalizeFunction should normalize the config, undefined 1`] = `undefined`; + +exports[`normalizeFunction should normalize the config, untrimmed name, zod schema, async handler 1`] = ` +[ + "loremIpsum ", + { + "description": "lorem ipsum", + "inputSchema": "isZod = true", + }, + [Function], +] +`; + +exports[`normalizeObject should normalize the config, basic 1`] = ` +{ + "original": { + "description": "lorem ipsum", + "handler": [Function], + "inputSchema": { + "properties": {}, + "type": "object", + }, + "name": "loremIpsum", + }, + "toolName": "loremIpsum", + "type": "object", + "value": [Function], +} +`; + +exports[`normalizeObject should normalize the config, missing handler 1`] = `undefined`; + +exports[`normalizeObject should normalize the config, missing schema 1`] = ` +{ + "error": "Tool "dolorSit" failed to set inputSchema. Provide a Zod schema, a Zod raw shape, or a plain JSON Schema object.", + "original": { + "description": "x", + "handler": [Function], + "name": "dolorSit", + }, + "toolName": "dolorSit", + "type": "invalid", + "value": [Function], +} +`; + +exports[`normalizeObject should normalize the config, null 1`] = `undefined`; + +exports[`normalizeObject should normalize the config, undefined 1`] = `undefined`; + +exports[`normalizeObject should normalize the config, untrimmed name, zod schema, async handler 1`] = ` +{ + "original": { + "description": "lorem ipsum", + "handler": [Function], + "inputSchema": "isZod = true", + "name": "loremIpsum", + }, + "toolName": "loremIpsum", + "type": "object", + "value": [Function], +} +`; + +exports[`normalizeTools should normalize configs, a creator 1`] = ` +[ + { + "index": 0, + "toolName": undefined, + "type": "creator", + }, +] +`; + +exports[`normalizeTools should normalize configs, an object 1`] = ` +[ + { + "index": 0, + "toolName": "loremIpsum", + "type": "object", + }, +] +`; + +exports[`normalizeTools should normalize configs, array of creators 1`] = ` +[ + { + "index": 0, + "toolName": undefined, + "type": "creator", + }, + { + "index": 1, + "toolName": undefined, + "type": "creator", + }, +] +`; + +exports[`normalizeTools should normalize configs, invalid file URLs, hostname, encoding 1`] = ` +[ + { + "index": 0, + "toolName": undefined, + "type": "invalid", + }, + { + "index": 1, + "toolName": undefined, + "type": "invalid", + }, +] +`; + +exports[`normalizeTools should normalize configs, mix of non-configs 1`] = ` +[ + { + "index": 0, + "toolName": undefined, + "type": "invalid", + }, + { + "index": 1, + "toolName": undefined, + "type": "invalid", + }, + { + "index": 2, + "toolName": undefined, + "type": "invalid", + }, + { + "index": 3, + "toolName": undefined, + "type": "invalid", + }, +] +`; + +exports[`normalizeTools should normalize configs, mix of package, object, tuple, creator 1`] = ` +[ + { + "index": 0, + "toolName": undefined, + "type": "package", + }, + { + "index": 1, + "toolName": "ametDolor", + "type": "object", + }, + { + "index": 2, + "toolName": "loremIpsum", + "type": "tuple", + }, + { + "index": 3, + "toolName": undefined, + "type": "creator", + }, +] +`; + +exports[`normalizeTools should normalize configs, single tuple 1`] = ` +[ + { + "index": 0, + "toolName": "loremIpsum", + "type": "tuple", + }, +] +`; + +exports[`normalizeTuple should normalize the config, basic 1`] = ` +{ + "original": [ + "loremIpsum", + { + "description": "lorem ipsum", + "inputSchema": { + "properties": {}, + "type": "object", + }, + }, + [Function], + ], + "toolName": "loremIpsum", + "type": "tuple", + "value": [Function], +} +`; + +exports[`normalizeTuple should normalize the config, missing handler 1`] = `undefined`; + +exports[`normalizeTuple should normalize the config, missing schema 1`] = ` +{ + "error": "Tool "dolorSit" failed to set inputSchema. Provide a Zod schema, a Zod raw shape, or a plain JSON Schema object.", + "original": [ + "dolorSit", + { + "description": "x", + }, + [Function], + ], + "toolName": "dolorSit", + "type": "invalid", + "value": [Function], +} +`; + +exports[`normalizeTuple should normalize the config, null 1`] = `undefined`; + +exports[`normalizeTuple should normalize the config, undefined 1`] = `undefined`; + +exports[`normalizeTuple should normalize the config, untrimmed name, zod schema, async handler 1`] = ` +{ + "original": [ + "loremIpsum ", + { + "description": "lorem ipsum", + "inputSchema": "isZod = true", + }, + [Function], + ], + "toolName": "loremIpsum", + "type": "tuple", + "value": [Function], +} +`; + +exports[`normalizeTupleSchema should normalize object, non-object 1`] = `undefined`; + +exports[`normalizeTupleSchema should normalize object, object missing inputSchema 1`] = `undefined`; + +exports[`normalizeTupleSchema should normalize object, valid JSON schema with description 1`] = ` +{ + "description": "hello", + "inputSchema": "isZod = true", +} +`; + +exports[`normalizeTupleSchema should normalize object, valid JSON schema without description 1`] = ` +{ + "inputSchema": "isZod = true", +} +`; diff --git a/src/__tests__/server.helpers.test.ts b/src/__tests__/server.helpers.test.ts index e15c250..a232c8b 100644 --- a/src/__tests__/server.helpers.test.ts +++ b/src/__tests__/server.helpers.test.ts @@ -1,4 +1,4 @@ -import { freezeObject, generateHash, hashCode, isPlainObject, isPromise, mergeObjects } from '../server.helpers'; +import { freezeObject, generateHash, hashCode, isPlainObject, isPromise, isReferenceLike, mergeObjects } from '../server.helpers'; describe('freezeObject', () => { it.each([ @@ -430,6 +430,93 @@ describe('isPlainObject', () => { }); }); +describe('isReferenceLike', () => { + it.each([ + { + description: 'string', + param: 'lorem', + value: false + }, + { + description: 'number', + param: 10_0000, + value: false + }, + { + description: 'plain object, empty', + param: {}, + value: true + }, + { + description: 'plain object', + param: { 1: 'lorem', 2: 'ipsum' }, + value: true + }, + { + description: 'create object', + param: Object.create(null), + value: true + }, + { + description: 'array', + param: [], + value: true + }, + { + description: 'null', + param: null, + value: false + }, + { + description: 'undefined', + param: undefined, + value: false + }, + { + description: 'NaN', + param: NaN, + value: false + }, + { + description: 'function', + param: () => 'lorem', + value: true + }, + { + description: 'date', + param: new Date('2023-01-01'), + value: true + }, + { + description: 'symbol', + param: Symbol('lorem ipsum'), + value: false + }, + { + description: 'error', + param: new Error('lorem ipsum'), + value: true + }, + { + description: 'regex', + param: /lorem/g, + value: true + }, + { + description: 'map', + param: new Map([['lorem', 1], ['ipsum', 2]]), + value: true + }, + { + description: 'set', + param: new Set([1, 2, 3]), + value: true + } + ])('should determine a non-primitive for $description', ({ param, value }) => { + expect(isReferenceLike(param)).toBe(value); + }); +}); + describe('mergeObjects', () => { it.each([ { diff --git a/src/__tests__/server.tools.test.ts b/src/__tests__/server.tools.test.ts new file mode 100644 index 0000000..e0f6b58 --- /dev/null +++ b/src/__tests__/server.tools.test.ts @@ -0,0 +1,779 @@ +import { resolve } from 'node:path'; +import { spawn } from 'child_process'; +import { log } from '../logger'; +import { + getBuiltInToolName, + computeFsReadAllowlist, + logWarningsErrors, + getFilePackageToolModules, + debugChild, + spawnToolsHost, + makeProxyCreators, + sendToolsHostShutdown, + composeTools +} from '../server.tools'; +import { awaitIpc, makeId, send } from '../server.toolsIpc'; +import { isZodSchema } from '../server.schema'; + +jest.mock('node:child_process', () => ({ + spawn: jest.fn() +})); + +jest.mock('../logger', () => ({ + log: { + warn: jest.fn(), + error: jest.fn(), + info: jest.fn(), + debug: jest.fn() + }, + formatUnknownError: jest.fn((error: unknown) => String(error)) +})); + +jest.mock('../server.toolsIpc', () => ({ + send: jest.fn(), + awaitIpc: jest.fn(), + makeId: jest.fn(() => 'id-1'), + isHelloAck: jest.fn((msg: any) => msg?.t === 'hello:ack'), + isInvokeResult: jest.fn((msg: any) => msg?.t === 'invoke:result'), + isLoadAck: jest.fn((id: string) => (msg: any) => msg?.t === 'load:ack' && msg?.id === id), + isManifestResult: jest.fn((id: string) => (msg: any) => msg?.t === 'manifest:result' && msg?.id === id) +})); + +describe('getBuiltInToolName', () => { + it('should return built-in tool name', () => { + const toolName = 'loremIpsum'; + const creator = () => {}; + + creator.toolName = toolName; + + expect(getBuiltInToolName(creator as any)).toBe(toolName.toLowerCase()); + }); +}); + +describe('computeFsReadAllowlist', () => { + it.each([ + { + description: 'with no tool modules', + options: { + toolModules: [], + contextUrl: 'file://', + contextPath: '/' + }, + expected: ['/'] + }, + { + description: 'with tool modules', + options: { + toolModules: ['@scope/pkg', resolve(process.cwd(), 'package.json')], + contextUrl: 'file://', + contextPath: '/' + }, + expected: ['/'] + }, + { + description: 'with missing context path', + options: { + toolModules: ['@scope/pkg', resolve(process.cwd(), 'package.json')], + contextUrl: 'file://', + contextPath: undefined + }, + expected: [] + } + ])('should return a list of allowed paths, $description', ({ options, expected }) => { + expect(computeFsReadAllowlist(options as any)).toEqual(expected); + }); +}); + +describe('logWarningsErrors', () => { + const MockLog = jest.mocked(log); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'with warnings only', + warnings: ['Warning 1', 'Warning 2'], + errors: [] + }, + { + description: 'with errors only', + warnings: [], + errors: ['Error 1', 'Error 2'] + }, + { + description: 'with both warnings and errors', + warnings: ['Warning 1'], + errors: ['Error 1'] + }, + { + description: 'with empty arrays', + warnings: [], + errors: [] + }, + { + description: 'with undefined warnings and errors', + warnings: undefined, + errors: undefined + }, + { + description: 'with single warning', + warnings: ['Single warning'], + errors: [] + }, + { + description: 'with single error', + warnings: [], + errors: ['Single error'] + } + ])('should log warnings and errors, $description', ({ warnings, errors }) => { + logWarningsErrors({ warnings, errors } as any); + + expect(MockLog.warn.mock.calls).toMatchSnapshot(); + }); +}); + +describe('getFilePackageToolModules,', () => { + it('should return filtered tool modules', () => { + const toolModules = [ + '@scope/pkg', + 'file:///test/module.js', + undefined, + 'http://example.com/module.js', + 'https://example.com/module.js' + ]; + const updated = getFilePackageToolModules({ toolModules } as any); + + expect(updated.length).toBe(4); + expect(updated).toMatchSnapshot(); + }); +}); + +describe('debugChild', () => { + let debugSpy: jest.SpyInstance; + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + debugSpy = jest.spyOn(log, 'debug').mockImplementation(() => {}); + warnSpy = jest.spyOn(log, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it.each([ + { + description: 'default', + message: 'lorem ipsum dolor sit amet' + }, + { + description: 'access denied', + message: 'Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet' + }, + { + description: 'access denied, multiple lines', + message: 'Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet\nError [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet' + }, + { + description: 'access denied, alt messaging', + message: 'Error [ERR_ACCESS_DENIED]: fs.readFileSync access is denied by permission model: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet\nError [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet' + }, + { + description: 'module not found', + message: 'Error [ERR_MODULE_NOT_FOUND]: Cannot find module \'/lorem/ipsum/dolor/sit/amet\' imported from /test/path' + }, + { + description: 'module not found, multiple lines', + message: 'Error [ERR_MODULE_NOT_FOUND]: Cannot find module \'/lorem/ipsum/dolor/sit/amet\' imported from /test/path\nError [ERR_MODULE_NOT_FOUND]: Cannot find module \'/lorem/ipsum/dolor/sit/amet\' imported from /test/path' + }, + { + description: 'generic multiline error', + message: 'Lorem ipsum\ndolor sit\namet' + }, + { + description: 'generic multiline error with spaces', + message: 'Lorem ipsum \n\tdolor sit\n amet' + }, + { + description: 'empty string', + message: '' + } + ])('should attempt to highlight specific messages, $description', async ({ message }) => { + let mockHandler: any; + const mockOff = jest.fn(); + const mockChild = { + pid: 123, + stderr: { + on: (_: any, handler: any) => mockHandler = handler, + off: mockOff + } + } as any; + + const unsubscribe = debugChild(mockChild, { sessionId: '1234567890' } as any); + + mockHandler(message); + + expect({ + warn: warnSpy.mock.calls, + debug: debugSpy.mock.calls + }).toMatchSnapshot(); + + unsubscribe(); + expect(mockOff).toHaveBeenCalledWith('data', mockHandler); + }); +}); + +describe('spawnToolsHost', () => { + const MockSpawn = jest.mocked(spawn); + const MockAwaitIpc = jest.mocked(awaitIpc); + const MockSend = jest.mocked(send); + const MockMakeId = jest.mocked(makeId); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'with undefined pluginIsolation, node 22', + options: { nodeVersion: 22, pluginIsolation: undefined } + }, + { + description: 'with strict pluginIsolation, node 22', + options: { nodeVersion: 22, pluginIsolation: 'strict' } + }, + { + description: 'with no pluginIsolation, node 24', + options: { nodeVersion: 24, pluginIsolation: 'none' } + }, + { + description: 'with strict pluginIsolation, node 24', + options: { nodeVersion: 24, pluginIsolation: 'strict' } + } + ])('attempt to spawn the Tools Host, $description', async ({ options }) => { + const updatedOptions = { pluginHost: { loadTimeoutMs: 10, invokeTimeoutMs: 10 }, ...options }; + const mockPid = 123; + const mockTools = [{ name: 'alphaTool' }, { name: 'betaTool' }]; + + MockSpawn.mockReturnValue({ + pid: mockPid + } as any); + + MockAwaitIpc + .mockResolvedValueOnce({ t: 'hello:ack', id: 'id-1' } as any) + .mockResolvedValueOnce({ t: 'load:ack', id: 'id-1', warnings: [], errors: [] } as any) + .mockResolvedValueOnce({ t: 'manifest:result', id: 'id-1', tools: mockTools } as any); + + const result = await spawnToolsHost(updatedOptions as any); + + expect(result.child.pid).toBe(mockPid); + expect(result.tools).toEqual(mockTools); + expect(MockMakeId).toHaveBeenCalledTimes(3); + expect(MockSend).toHaveBeenCalledTimes(3); + + expect({ + spawn: MockSpawn.mock.calls?.[0]?.slice?.(1) + }).toMatchSnapshot('spawn'); + }); + + it('should throw when resolve fails', async () => { + process.env.NODE_ENV = '__test__'; + + await expect( + spawnToolsHost({ nodeVersion: 24, pluginIsolation: 'strict', pluginHost: {} } as any) + ).rejects.toThrow(/Failed to resolve Tools Host/); + + process.env.NODE_ENV = 'local'; + }); +}); + +describe('makeProxyCreators', () => { + const MockAwaitIpc = jest.mocked(awaitIpc); + const MockSend = jest.mocked(send); + const MockMakeId = jest.mocked(makeId); + const MockLog = jest.mocked(log); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'no tools', + tools: [] + }, + { + description: 'basic', + tools: [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum', + description: 'Lorem ipsum dolor sit amet', + inputSchema: {}, + source: '' + } + ] + }, + { + description: 'null JSON input schema', + tools: [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum', + description: 'Lorem ipsum dolor sit amet', + inputSchema: null, + source: '' + } + ] + }, + { + description: 'undefined JSON input schema', + tools: [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum', + description: 'Lorem ipsum dolor sit amet', + inputSchema: undefined, + source: '' + } + ] + } + ])('should attempt to return proxy creators, a function wrapper per tool, $description', ({ tools }) => { + const proxies = makeProxyCreators({ tools } as any, { pluginHost: { invokeTimeoutMs: 10 } } as any); + const output = proxies.map(proxy => { + const [name, { description, inputSchema, ...rest }, handler] = proxy(); + + return [ + name, + { description, inputSchema: `isZod = ${isZodSchema(inputSchema)}`, ...rest }, + handler + ]; + }); + + expect({ + output, + debug: MockLog.debug.mock.calls + }).toMatchSnapshot(); + }); + + it.each([ + { + description: 'ok false', + response: { + ok: false, + result: { value: 7 } + } + }, + { + description: 'ok false with error', + response: { + ok: false, + result: { value: 7 }, + error: { message: 'Error message' } + } + }, + { + description: 'ok false with full error', + response: { + ok: false, + result: { value: 7 }, + error: { message: 'Error message', stack: 'line 1\nline 2', code: 'ERR_CODE', cause: { details: 'Details' } } + } + } + ])('should attempt to invoke a creator then throw an error on child response, $description', async ({ response }) => { + const tools = [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum', + description: 'Lorem ipsum dolor sit amet', + inputSchema: {}, + source: '' + } + ]; + + MockMakeId.mockReturnValue('id-1' as any); + MockAwaitIpc + .mockResolvedValueOnce({ t: 'invoke:result', id: 'id-1', ...response } as any); + + const proxies = makeProxyCreators({ tools } as any, { pluginHost: { invokeTimeoutMs: 10 } } as any); + const [_name, _schema, handler]: any = proxies.map(proxy => { + const [name, { description, inputSchema, ...rest }, handler] = proxy(); + + return [ + name, + { description, inputSchema: `isZod = ${isZodSchema(inputSchema)}`, ...rest }, + handler + ]; + })[0]; + + await expect(handler({ loremIpsum: 7 })).rejects.toMatchSnapshot('handler'); + expect(MockSend.mock.calls).toMatchSnapshot('send'); + }); +}); + +describe('sendToolsHostShutdown', () => { + const MockLog = jest.mocked(log); + const MockSend = jest.mocked(send); + let mapGetSpy: jest.SpyInstance; + let mapDeleteSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + mapGetSpy = jest.spyOn(Map.prototype, 'get'); + mapDeleteSpy = jest.spyOn(Map.prototype, 'delete'); + }); + + afterEach(() => { + jest.useRealTimers(); + mapGetSpy.mockRestore(); + mapDeleteSpy.mockRestore(); + }); + + it('should attempt graceful shutdown of child', async () => { + const onceHandlers: Record = {}; + const child = { + kill: jest.fn(), + killed: false, + once: jest.fn((event: string, handler: any) => { + onceHandlers[event] = handler; + }), + off: jest.fn(), + stderr: { + on: jest.fn(), + off: jest.fn() + } + }; + const handle = { child, closeStderr: jest.fn() }; + const sessionId = 'test-session-id'; + + mapGetSpy.mockReturnValue(handle); + + const promise = sendToolsHostShutdown({ pluginHost: { gracePeriodMs: 10 } } as any, { sessionId } as any); + + onceHandlers['disconnect'](); + + await promise; + + expect(MockSend).toHaveBeenCalledTimes(1); + expect(child.once).toHaveBeenCalledTimes(2); + expect(child.off).toHaveBeenCalledWith('exit', onceHandlers['exit']); + expect(child.off).toHaveBeenCalledWith('disconnect', onceHandlers['disconnect']); + expect(handle.closeStderr).toHaveBeenCalledTimes(1); + expect(mapDeleteSpy).toHaveBeenCalledWith(sessionId); + + jest.advanceTimersByTime(220); + expect(child.kill).not.toHaveBeenCalled(); + }); + + it('should attempt force shutdown of child', async () => { + const child = { + // eslint-disable-next-line func-names + kill: jest.fn(function (this: any) { + this.killed = true; + + return true; + }), + killed: false, + once: jest.fn(), + off: jest.fn(), + stderr: { + on: jest.fn(), + off: jest.fn() + } + }; + const handle = { child, closeStderr: jest.fn() }; + const sessionId = 'test-session-id'; + + mapGetSpy.mockReturnValue(handle); + + const promise = sendToolsHostShutdown({ pluginHost: { gracePeriodMs: 10 } } as any, { sessionId } as any); + + jest.advanceTimersByTime(20); + await promise; + + jest.advanceTimersByTime(220); + + expect(MockSend).toHaveBeenCalledTimes(1); + expect(child.once).toHaveBeenCalledTimes(2); + expect(child.kill).toHaveBeenCalledTimes(1); + expect(child.kill).toHaveBeenCalledWith('SIGKILL'); + expect(child.killed).toBe(true); + expect(child.off).toHaveBeenCalledTimes(2); + expect(handle.closeStderr).toHaveBeenCalledTimes(1); + expect(mapDeleteSpy).toHaveBeenCalledWith(sessionId); + }); + + it('should attempt force shutdown of child and fail', async () => { + const child = { + kill: jest.fn() + .mockImplementationOnce(() => { + throw new Error('Mock failed to kill child process'); + }) + // eslint-disable-next-line func-names + .mockImplementationOnce(function (this: any) { + this.killed = true; + + return true; + }), + killed: false, + once: jest.fn(), + off: jest.fn(), + stderr: { + on: jest.fn(), + off: jest.fn() + } + }; + const handle = { + child, + closeStderr: jest.fn() + .mockImplementationOnce(() => { + throw new Error('Mock close failure 1'); + }) + .mockImplementationOnce(() => {}) + }; + const sessionId = 'test-session-id'; + + MockSend.mockImplementationOnce(() => { + throw new Error('Mock send failure'); + }); + + mapGetSpy.mockReturnValue(handle); + + const promise = sendToolsHostShutdown({ pluginHost: { gracePeriodMs: 10 } } as any, { sessionId } as any); + + jest.advanceTimersByTime(10); + await promise; + + jest.advanceTimersByTime(220); + + expect(child.kill).toHaveBeenCalledWith('SIGKILL'); + expect(child.killed).toBe(false); + expect([...MockLog.error.mock.calls, ...MockLog.warn.mock.calls, ...MockLog.info.mock.calls]).toMatchSnapshot(); + }); +}); + +describe('composeTools', () => { + const MockSpawn = jest.mocked(spawn); + const MockAwaitIpc = jest.mocked(awaitIpc); + const MockLog = jest.mocked(log); + + // Mock default creators + const loremIpsum = () => ['loremIpsum', { description: 'lorem ipsum', inputSchema: {} }, () => {}]; + const dolorSitAmet = () => ['dolorSitAmet', { description: 'dolor sit amet', inputSchema: {} }, () => {}]; + const consecteturAdipiscingElit = () => ['consecteturAdipiscingElit', { description: 'consectetur adipiscing elit', inputSchema: {} }, () => {}]; + + loremIpsum.toolName = 'loremIpsum'; + dolorSitAmet.toolName = 'dolorSitAmet'; + consecteturAdipiscingElit.toolName = 'consecteturAdipiscingElit'; + + beforeEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + }); + + afterAll(() => { + }); + + it.each([ + { + description: 'default package creators', + nodeVersion: 22, + modules: [], + expectedModuleCount: 3 + }, + { + description: 'invalid creator', + nodeVersion: 22, + modules: [ + { name: 'dolor', error: 'Creator error message' } + ], + expectedModuleCount: 3 + }, + { + description: 'inline creators', + nodeVersion: 22, + modules: [ + () => ['lorem', { description: 'ipsum', inputSchema: {} }, () => {}], + { name: 'dolor', description: 'sit amet', inputSchema: {}, handler: () => {} } + ], + expectedModuleCount: 5 + }, + { + description: 'inline duplicate creators', + nodeVersion: 22, + modules: [ + () => ['lorem', { description: 'ipsum', inputSchema: {} }, () => {}], + { name: 'dolor', description: 'sit amet', inputSchema: {}, handler: () => {} }, + { name: 'dolor', description: 'sit amet', inputSchema: {}, handler: () => {} } + ], + expectedModuleCount: 5 + }, + { + description: 'file package creators', + nodeVersion: 22, + modules: ['file:///test/module.js', '@patternfly/tools'], + expectedModuleCount: 5 + }, + { + description: 'file package duplicate creators', + nodeVersion: 22, + modules: ['file:///test/module.js', '@patternfly/tools', '@patternfly/tools'], + expectedModuleCount: 5 + }, + { + description: 'file package creators, Node.js 20', + nodeVersion: 20, + modules: ['file:///test/module.js', '@patternfly/tools'], + expectedModuleCount: 3 + }, + { + description: 'file package creators, Node.js 24', + nodeVersion: 24, + modules: ['file:///test/module.js', '@patternfly/tools'], + expectedModuleCount: 5 + }, + { + description: 'file package creators, Node.js undefined', + nodeVersion: undefined, + modules: ['file:///test/module.js', '@patternfly/tools'], + expectedModuleCount: 3 + }, + { + description: 'inline and file package creators', + nodeVersion: 22, + modules: [ + () => ['lorem', { description: 'ipsum', inputSchema: {} }, () => {}], + { name: 'dolor', description: 'sit amet', inputSchema: {}, handler: () => {} }, + 'file:///test/module.js', + '@patternfly/tools' + ], + expectedModuleCount: 7 + }, + { + description: 'inline and file package creators duplicate builtin creators', + nodeVersion: 22, + modules: [ + ['loremIpsum', { description: 'lorem ipsum', inputSchema: {} }, () => {}], + 'dolorSitAmet' + ], + expectedModuleCount: 3 + }, + { + description: 'inline and file package creators, duplicates', + nodeVersion: 22, + modules: [ + { name: '@patternfly/tools', description: 'lorem ipsum', inputSchema: {}, handler: () => {} }, + { name: 'dolor', description: 'sit amet', inputSchema: {}, handler: () => {} }, + 'file:///test/module.js', + '@patternfly/tools', + 'DOLOR ' + ], + expectedModuleCount: 6 + }, + { + description: 'inline and file package creators, duplicates, Node.js 20', + nodeVersion: 20, + modules: [ + { name: '@patternfly/tools', description: 'lorem ipsum', inputSchema: {}, handler: () => {} }, + { name: 'dolor', description: 'sit amet', inputSchema: {}, handler: () => {} }, + 'file:///test/module.js', + '@patternfly/tools', + 'DOLOR ' + ], + expectedModuleCount: 5 + } + ])('should attempt to setup creators, $description', async ({ modules, nodeVersion, expectedModuleCount }) => { + const mockChild = { + pid: 123, + once: jest.fn(), + off: jest.fn() + }; + const filePackageToolModules: any[] = modules; + const mockFilePackageTools = filePackageToolModules.filter(tool => typeof tool === 'string') + .map(name => ({ name, description: name, inputSchema: {}, source: name })); + + const sessionId = 'test-session-id'; + + MockSpawn.mockReturnValueOnce(mockChild as any); + + MockAwaitIpc + .mockResolvedValueOnce({ t: 'hello:ack', id: 'id-1' } as any) + .mockResolvedValueOnce({ t: 'load:ack', id: 'id-1', warnings: [], errors: [] } as any) + .mockResolvedValueOnce({ t: 'manifest:result', id: 'id-1', tools: mockFilePackageTools } as any); + + const defaultCreators: any[] = [loremIpsum, dolorSitAmet, consecteturAdipiscingElit]; + const globalOptions: any = { toolModules: filePackageToolModules, nodeVersion, contextUrl: 'file:///test/path', contextPath: '/test/path' }; + const sessionOptions: any = { sessionId }; + const tools = await composeTools(defaultCreators, globalOptions, sessionOptions); + + expect(tools.length).toBe(expectedModuleCount); + expect({ + toolsCount: tools.length, + log: MockLog.warn.mock.calls + }).toMatchSnapshot(); + }); + + it('should attempt to setup handlers for child exit, disconnect', async () => { + const onceHandlers: Record = {}; + const mockChild = { + pid: 123, + once: jest.fn((event: string, handler: any) => { + onceHandlers[event] = handler; + }), + off: jest.fn(), + stderr: { + on: jest.fn(), + off: jest.fn() + } + }; + const filePackageToolModules: any[] = ['file:///test/module.js', '@patternfly/woot']; + const mockFilePackageTools = filePackageToolModules.map(tool => ({ name: tool, description: tool, inputSchema: {}, source: tool })); + const sessionId = 'test-session-id'; + + MockSpawn.mockReturnValueOnce(mockChild as any); + + MockAwaitIpc + .mockResolvedValueOnce({ t: 'hello:ack', id: 'id-1' } as any) + .mockResolvedValueOnce({ t: 'load:ack', id: 'id-1', warnings: [], errors: [] } as any) + .mockResolvedValueOnce({ t: 'manifest:result', id: 'id-1', tools: mockFilePackageTools } as any); + + const defaultCreators: any[] = [loremIpsum, dolorSitAmet, consecteturAdipiscingElit]; + const globalOptions: any = { toolModules: filePackageToolModules, nodeVersion: 22, contextUrl: 'file:///test/path', contextPath: '/test/path' }; + const sessionOptions: any = { sessionId }; + + await composeTools(defaultCreators, globalOptions, sessionOptions); + + onceHandlers['disconnect'](); + + expect(mockChild.once).toHaveBeenCalledTimes(2); + expect(mockChild.stderr.on).toHaveBeenCalledWith('data', expect.any(Function)); + expect(mockChild.stderr.off).toHaveBeenCalledWith('data', expect.any(Function)); + expect(mockChild.off).toHaveBeenCalledWith('exit', onceHandlers['exit']); + expect(mockChild.off).toHaveBeenCalledWith('disconnect', onceHandlers['disconnect']); + }); + + it('should return default creators on tools host error', async () => { + const filePackageToolModules: any[] = ['@patternfly/tools']; + + const sessionId = 'test-session-id'; + + MockSpawn.mockImplementationOnce(() => { + throw new Error('Mock spawn failure'); + }); + + const defaultCreators: any[] = [loremIpsum, dolorSitAmet, consecteturAdipiscingElit]; + const globalOptions: any = { toolModules: filePackageToolModules, nodeVersion: 22, contextUrl: 'file:///test/path', contextPath: '/test/path' }; + const sessionOptions: any = { sessionId }; + const tools = await composeTools(defaultCreators, globalOptions, sessionOptions); + + expect(tools.length).toBe(defaultCreators.length); + expect({ + toolsCount: tools.length, + log: MockLog.warn.mock.calls + }).toMatchSnapshot(); + }); +}); diff --git a/src/__tests__/server.toolsUser.test.ts b/src/__tests__/server.toolsUser.test.ts new file mode 100644 index 0000000..2e180af --- /dev/null +++ b/src/__tests__/server.toolsUser.test.ts @@ -0,0 +1,642 @@ +import { pathToFileURL } from 'node:url'; +import { basename, resolve } from 'node:path'; +import { z } from 'zod'; +import { + createMcpTool, + isFilePath, + isUrlLike, + normalizeFilePackage, + normalizeFilePath, + normalizeFileUrl, + normalizeTuple, + normalizeTupleSchema, + normalizeObject, + normalizeFunction, + normalizeTools, + sanitizeDataProp, + sanitizePlainObject +} from '../server.toolsUser'; +import { isZodSchema } from '../server.schema'; + +describe('sanitizeDataProp', () => { + it('returns descriptor for data property and excludes accessors', () => { + const obj = { a: 1 }; + + Object.defineProperty(obj, 'b', { get: () => 2 }); + const a = sanitizeDataProp(obj, 'a'); + const b = sanitizeDataProp(obj, 'b'); + const cProp = sanitizeDataProp(obj, 'c'); + + expect(a?.value).toBe(1); + expect(b).toBeUndefined(); + expect(cProp).toBeUndefined(); + }); +}); + +describe('sanitizePlainObject', () => { + it('filters to allowed keys and ignores accessors', () => { + const allowed = new Set(['x', 'y']); + const obj = { x: 1, y: 2, z: 3 }; + + Object.defineProperty(obj, 'y', { get: () => 2 }); + const out = sanitizePlainObject(obj, allowed); + + expect(out).toEqual({ x: 1 }); + expect(Object.prototype.hasOwnProperty.call(out, 'y')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(out, 'z')).toBe(false); + }); + + it.each([ + { description: 'null', obj: null }, + { description: 'undefined', obj: undefined }, + { description: 'array', obj: [1, 2, 3] }, + { description: 'function', obj: () => {} } + ])('should return an empty object, $description', ({ obj }) => { + expect(sanitizePlainObject(obj, new Set())).toEqual({}); + }); +}); + +describe('isFilePath', () => { + it.each([ + { description: 'absolute path', file: '/path/to/file.txt' }, + { description: 'absolute path ref no extension', file: '/path/to/another/file' }, + { description: 'min file extension', file: 'path/to/another/file.y' }, + { description: 'potential multiple extensions', file: 'path/to/another/file.test.js' }, + { description: 'current dir ref', file: './path/to/another/file.txt' }, + { description: 'parent dir ref', file: '../path/to/another/file.txt' } + ])('should validate $description', ({ file }) => { + expect(isFilePath(file)).toBe(true); + }); + + it.each([ + { description: 'no file extension or dir ref', file: 'path/to/another/file' } + ])('should fail, $description', ({ file }) => { + expect(isFilePath(file)).toBe(false); + }); +}); + +describe('isUrlLike', () => { + it.each([ + { description: 'http', url: 'http://example.com' }, + { description: 'https', url: 'https://example.com' }, + { description: 'file', url: 'file:///path/to/file.txt' }, + { description: 'node', url: 'node://path/to/file.txt' }, + { description: 'data', url: 'data:text/plain;base64,1234567890==' } + ])('should validate $description', ({ url }) => { + expect(isUrlLike(url)).toBe(true); + }); + + it.each([ + { description: 'invalid protocol', url: 'ftp://example.com' }, + { description: 'random', url: 'random://example.com' }, + { description: 'null', url: null }, + { description: 'undefined', url: undefined } + ])('should fail, $description', ({ url }) => { + expect(isUrlLike(url as any)).toBe(false); + }); +}); + +describe('normalizeTupleSchema', () => { + it.each([ + { + description: 'valid JSON schema with description', + schema: { description: ' hello ', inputSchema: { type: 'object', properties: {} } } + }, + { + description: 'valid JSON schema without description', + schema: { inputSchema: { type: 'object', properties: {} } } + }, + { + description: 'non-object', + schema: 'nope' + }, + { + description: 'object missing inputSchema', + schema: { description: 'x' } + } + ])('should normalize object, $description', ({ schema }) => { + const updated = normalizeTupleSchema(schema); + + if (updated?.inputSchema) { + updated.inputSchema = `isZod = ${isZodSchema(updated.inputSchema)}`; + } + + expect(updated).toMatchSnapshot(); + }); + + it('should have a memo property', () => { + expect(normalizeTupleSchema.memo).toBeDefined(); + }); +}); + +describe('normalizeTuple', () => { + it.each([ + { + description: 'basic', + tuple: ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}] + }, + { + description: 'untrimmed name, zod schema, async handler', + tuple: ['loremIpsum ', { description: 'lorem ipsum', inputSchema: z.any() }, async () => {}] + }, + { + description: 'missing schema', + tuple: ['dolorSit', { description: 'x' }, async () => {}] + }, + { + description: 'missing handler', + tuple: ['dolorSit', { description: 'x' }] + }, + { + description: 'undefined', + tuple: undefined + }, + { + description: 'null', + tuple: null + } + ])('should normalize the config, $description', ({ tuple }) => { + const updated = normalizeTuple(tuple); + + if ((updated?.original as any)?.[1]?.inputSchema && isZodSchema((updated?.original as any)[1].inputSchema)) { + (updated?.original as any)[1].inputSchema = 'isZod = true'; + } + + expect(updated).toMatchSnapshot(); + }); + + it('should have a memo property', () => { + expect(normalizeTuple.memo).toBeDefined(); + }); +}); + +describe('normalizeObject', () => { + it.each([ + { + description: 'basic', + obj: { name: 'loremIpsum', description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} }, handler: () => {} } + }, + { + description: 'untrimmed name, zod schema, async handler', + obj: { name: 'loremIpsum', description: 'lorem ipsum', inputSchema: z.any(), handler: async () => {} } + }, + { + description: 'missing schema', + obj: { name: 'dolorSit', description: 'x', handler: async () => {} } + }, + { + description: 'missing handler', + obj: { name: 'dolorSit', description: 'x' } + }, + { + description: 'undefined', + tuple: undefined + }, + { + description: 'null', + tuple: null + } + ])('should normalize the config, $description', ({ obj }) => { + const updated = normalizeObject(obj); + + if ((updated?.original as any)?.inputSchema && isZodSchema((updated?.original as any).inputSchema)) { + (updated?.original as any).inputSchema = 'isZod = true'; + } + + expect(updated).toMatchSnapshot(); + }); + + it('should have a memo property', () => { + expect(normalizeObject.memo).toBeDefined(); + }); +}); + +describe('normalizeFunction', () => { + it.each([ + { + description: 'basic', + func: () => ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}] + }, + { + description: 'untrimmed name, zod schema, async handler', + func: () => ['loremIpsum ', { description: 'lorem ipsum', inputSchema: z.any() }, async () => {}] + }, + { + description: 'missing schema', + func: () => ['dolorSit', { description: 'x' }, async () => {}] + }, + { + description: 'missing handler', + func: () => ['dolorSit', { description: 'x' }] + }, + { + description: 'undefined', + func: () => undefined + }, + { + description: 'null', + func: () => null + } + ])('should normalize the config, $description', ({ func }) => { + const out = normalizeFunction(func); + const updated = (out?.original as any)?.(); + + if (updated?.[1]?.inputSchema && isZodSchema(updated[1].inputSchema)) { + updated[1].inputSchema = 'isZod = true'; + } + + expect(updated).toMatchSnapshot(); + }); + + it('should throw a predictable error on unwrap if the function errors', () => { + const func = () => { + throw new Error('Function error'); + }; + + const updated = normalizeFunction(func); + + expect(() => (updated?.value as any)?.()).toThrow('Tool failed to load:'); + }); + + it('should have a memo property', () => { + expect(normalizeFunction.memo).toBeDefined(); + }); +}); + +describe('normalizeFileUrl', () => { + it.each([ + { + description: 'file URL', + file: pathToFileURL(resolve(process.cwd(), 'package.json')).href, + expectType: 'file' + }, + { + description: 'relative file path', + file: './package.json', + expectType: undefined + }, + { + description: 'absolute file path', + file: resolve(process.cwd(), 'package.json'), + expectType: undefined + }, + { + description: 'package name string', + file: '@scope/pkg', + expectType: undefined + }, + { + description: 'http URL (not file)', + file: 'https://github.com/patternfly/patternfly-mcp/module.mjs', + expectType: undefined + }, + { + description: 'invalid file URLs, hostname', + config: 'file://example.com/etc/hosts', + expectType: undefined + }, + { + description: 'invalid file URLs, encoding', + file: 'file:///foo/%E0%A4%A', + expectType: 'invalid' + } + ])('handles $description', ({ file, expectType }) => { + const updated = normalizeFileUrl(file); + + if (updated) { + updated.fsReadDir = '/'; + updated.normalizedUrl = `/${basename(updated.normalizedUrl as string)}`; + updated.original = `/${basename(updated.original as string)}`; + updated.value = `/${basename(updated.value as string)}`; + + if (updated.error) { + updated.error = 'true'; + } + } + expect(updated?.type).toBe(expectType); + expect(updated).toMatchSnapshot(); + }); + + it('should have a memo property', () => { + expect(normalizeFileUrl.memo).toBeDefined(); + }); +}); + +describe('normalizeFilePath', () => { + it.each([ + { + description: 'file URL', + file: pathToFileURL(resolve(process.cwd(), 'package.json')).href, + expectType: undefined + }, + { + description: 'relative file path', + file: './package.json', + expectType: 'file' + }, + { + description: 'absolute file path', + file: resolve(process.cwd(), 'package.json'), + expectType: 'file' + }, + { + description: 'package name string', + file: '@scope/pkg', + expectType: undefined + } + ])('handles $description', ({ file, expectType }) => { + const updated = normalizeFilePath(file); + + if (updated) { + updated.fsReadDir = '/'; + updated.normalizedUrl = `/${basename(updated.normalizedUrl as string)}`; + updated.original = `/${basename(updated.original as string)}`; + updated.value = `/${basename(updated.value as string)}`; + + if (updated.error) { + updated.error = 'true'; + } + } + + expect(updated?.type).toBe(expectType); + expect(updated).toMatchSnapshot(); + }); + + it('should have a memo property', () => { + expect(normalizeFilePath.memo).toBeDefined(); + }); + + it('should use memoization consistently for contextPath and contextUrl results', () => { + const config = './fixtures/tool.mjs'; + + const resultOne = normalizeFilePath.memo(config, { contextPath: '/A', contextUrl: 'file:///A/index.mjs' }); + const resultTwo = normalizeFilePath.memo(config, { contextPath: '/B', contextUrl: 'file:///B/index.mjs' }); + const resultThree = normalizeFilePath.memo(config, { contextUrl: 'file:///B/index.mjs', contextPath: '/B' }); + + expect(resultTwo).not.toEqual(resultOne); + expect(resultThree).toEqual(resultTwo); + }); +}); + +describe('normalizeFilePackage', () => { + it.each([ + { + description: 'file URL', + file: pathToFileURL(resolve(process.cwd(), 'package.json')).href, + expectType: 'file' + }, + { + description: 'relative file path', + file: './package.json', + expectType: 'file' + }, + { + description: 'absolute file path', + file: resolve(process.cwd(), 'package.json'), + expectType: 'file' + }, + { + description: 'package name string', + file: '@scope/pkg', + expectType: 'package' + }, + { + description: 'http URL (not file)', + file: 'https://github.com/patternfly/patternfly-mcp/module.mjs', + expectType: 'package' + }, + { + description: 'undefined', + file: undefined, + expectType: undefined + }, + { + description: 'null', + file: null, + expectType: undefined + }, + { + description: 'number', + file: 10_000, + expectType: undefined + }, + { + description: 'invalid file URLs, hostname', + config: 'file://example.com/etc/hosts', + expectType: undefined + }, + { + description: 'invalid file URLs, encoding', + file: 'file:///foo/%E0%A4%A', + expectType: 'invalid' + } + ])('handles $description', ({ file, expectType }) => { + const updated = normalizeFilePackage(file); + + if (updated) { + updated.fsReadDir = '/'; + updated.normalizedUrl = `/${basename(updated.normalizedUrl as string)}`; + updated.original = `/${basename(updated.original as string)}`; + updated.value = `/${basename(updated.value as string)}`; + + if (updated.error) { + updated.error = 'true'; + } + } + + expect(updated?.type).toBe(expectType); + expect(updated).toMatchSnapshot(); + }); + + it('should have a memo property', () => { + expect(normalizeFilePackage.memo).toBeDefined(); + }); + + it('should use memoization consistently for contextPath and contextUrl results', () => { + const config = './fixtures/tool.mjs'; + + const resultOne = normalizeFilePackage.memo(config, { contextPath: '/A', contextUrl: 'file:///A/index.mjs' }); + const resultTwo = normalizeFilePackage.memo(config, { contextPath: '/B', contextUrl: 'file:///B/index.mjs' }); + const resultThree = normalizeFilePackage.memo(config, { contextUrl: 'file:///B/index.mjs', contextPath: '/B' }); + + expect(resultTwo).not.toEqual(resultOne); + expect(resultThree).toEqual(resultTwo); + }); +}); + +describe('normalizeTools', () => { + it.each([ + { + description: 'a creator', + config: () => ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}] + }, + { + description: 'array of creators', + config: [ + () => ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}], + () => ['dolorSit', { description: 'dolor sit', inputSchema: { type: 'object', properties: {} } }, () => {}] + ] + }, + { + description: 'an object', + config: { name: 'loremIpsum', description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} }, handler: () => {} } + }, + { + description: 'mix of package, object, tuple, creator', + config: [ + '@scope/pkg', + { name: 'ametDolor', description: 'amet dolor', inputSchema: { type: 'object', properties: {} }, handler: () => {} }, + ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}], + () => ['dolorSit', { description: 'dolor sit', inputSchema: { type: 'object', properties: {} } }, () => {}] + ] + }, + { + description: 'single tuple', + config: ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}] + }, + { + description: 'mix of non-configs', + config: [null, undefined, { x: 1 }, new Error('lorem ipsum')] + }, + { + description: 'invalid file URLs, hostname, encoding', + config: ['file://example.com/etc/hosts', 'file:///foo/%E0%A4%A'] + } + ])('should normalize configs, $description', ({ config }) => { + const result = normalizeTools(config); + const configLength = !normalizeTuple(config) && Array.isArray(config) ? config.length : 1; + + expect(result.length).toBe(configLength); + expect(result.map(({ index, type, toolName }) => ({ index, type, toolName }))).toMatchSnapshot(); + }); + + it('should flatten when using non-tuple configs (arrays)', () => { + const config = [[1, 2, 3], ['lorem', 'ipsum', 'dolor', 'sit']]; + const result = normalizeTools(config); + const configLength = config.flat().length; + + expect(result.length).toBe(configLength); + }); + + it('should have a memo property', () => { + expect(normalizeTools.memo).toBeDefined(); + }); + + it.each([ + { + description: 'file', + config: './fixtures/tool.mjs' + }, + { + description: 'package', + config: '@scope/pkg' + }, + { + description: 'inline function', + config: () => ['a', { inputSchema: {} }, () => {}] + }, + { + description: 'array of inline function', + config: [() => ['a', { inputSchema: {} }, () => {}]] + }, + { + description: 'inline tuple', + config: ['a', { description: 'a', inputSchema: {} }, () => {}] + }, + { + description: 'inline object', + config: { name: 'a', description: 'a', inputSchema: {}, handler: () => {} } + }, + { + description: 'array of inline configurations', + config: [ + './fixtures/tool.mjs', + () => ['a', { inputSchema: {} }, () => {}], + { name: 'b', description: 'b', inputSchema: {}, handler: () => {} }, + ['c', { description: 'c', inputSchema: {} }, () => {}] + ] + } + ])('should use memoization consistently with contextPath and contextUrl results, $description', ({ config }) => { + const resultOne = normalizeTools.memo(config, { contextPath: '/A', contextUrl: 'file:///A/index.mjs' }); + const resultTwo = normalizeTools.memo(config, { contextPath: '/B', contextUrl: 'file:///B/index.mjs' }); + const resultThree = normalizeTools.memo(config, { contextUrl: 'file:///B/index.mjs', contextPath: '/B' }); + + expect(resultTwo).not.toBe(resultOne); + expect(resultThree).toBe(resultTwo); + }); +}); + +describe('createMcpTool', () => { + it.each([ + { + description: 'a creator', + config: () => ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}] + }, + { + description: 'array of creators', + config: [ + () => ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}], + () => ['dolorSit', { description: 'dolor sit', inputSchema: { type: 'object', properties: {} } }, () => {}] + ] + }, + { + description: 'an object', + config: { name: 'loremIpsum', description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} }, handler: () => {} } + }, + { + description: 'mix of package, object, tuple, creator', + config: [ + '@scope/pkg', + { name: 'ametDolor', description: 'amet dolor', inputSchema: { type: 'object', properties: {} }, handler: () => {} }, + ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}], + () => ['dolorSit', { description: 'dolor sit', inputSchema: { type: 'object', properties: {} } }, () => {}] + ] + }, + { + description: 'single tuple', + config: ['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}] + }, + { + description: 'nested createMcpTool calls', + config: [ + createMcpTool([createMcpTool('@scope/pkg1'), '@scope/pkg2', '@scope/pkg3']), + createMcpTool(createMcpTool(['loremIpsum', { description: 'lorem ipsum', inputSchema: { type: 'object', properties: {} } }, () => {}])), + createMcpTool(['dolorSit', { description: 'dolor sit', inputSchema: { type: 'object', properties: {} } }, () => {}]), + createMcpTool('@scope/pkg4'), + '@scope/pkg5' + ] + } + ])('should normalize configs, $description', ({ config }) => { + const result = createMcpTool(config); + + expect(result).toMatchSnapshot(); + }); + + it.each([ + { + description: 'packages, mix of non-configs', + config: ['@scope/pkg', '@scope/pkg2', '@scope/pkg3', [1, 2, 3], new Error('lorem ipsum')], + expectInvalidIndex: 3 + }, + { + description: 'undefined', + config: ['@scope/pkg', undefined], + expectInvalidIndex: 1 + } + ])('should throw an error, $description', ({ config, expectInvalidIndex }) => { + expect(() => createMcpTool(config)).toThrow(`createMcpTool: invalid configuration used at index ${expectInvalidIndex}:`); + }); + + it.each([ + { + description: 'invalid file path, hostname', + config: 'file://example.com/etc/hosts' + }, + { + description: 'invalid file path, bad encoding', + config: 'file:///foo/%E0%A4%A' + } + ])('should throw an error with invalid file paths, $description', ({ config }) => { + expect(() => createMcpTool([config])).toThrow('Failed to resolve file'); + }); +}); diff --git a/src/options.defaults.ts b/src/options.defaults.ts index 640389b..5aca992 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -1,6 +1,7 @@ import { basename, join, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; import packageJson from '../package.json'; +import { type ToolModule } from './server.toolsUser'; /** * Application defaults, not all fields are user-configurable @@ -18,6 +19,7 @@ import packageJson from '../package.json'; * @property {LoggingOptions} logging - Logging options. * @property name - Name of the package. * @property nodeVersion - Node.js major version. + * @property pluginIsolation - Isolation preset for external plugins. * @property {PluginHostOptions} pluginHost - Plugin host options. * @property repoName - Name of the repository. * @property pfExternal - PatternFly external docs URL. @@ -31,6 +33,8 @@ import packageJson from '../package.json'; * @property pfExternalAccessibility - PatternFly accessibility URL. * @property {typeof RESOURCE_MEMO_OPTIONS} resourceMemoOptions - Resource-level memoization options. * @property {typeof TOOL_MEMO_OPTIONS} toolMemoOptions - Tool-specific memoization options. + * @property {ToolModule|ToolModule[]} toolModules - Array of external tool modules (ESM specs or paths) to be loaded and + * registered with the server. * @property separator - Default string delimiter. * @property urlRegex - Regular expression pattern for URL matching. * @property version - Version of the package. @@ -46,6 +50,7 @@ interface DefaultOptions { logging: TLogOptions; name: string; nodeVersion: number; + pluginIsolation: 'none' | 'strict'; pluginHost: PluginHostOptions; pfExternal: string; pfExternalDesignComponents: string; @@ -60,6 +65,7 @@ interface DefaultOptions { resourceMemoOptions: Partial; separator: string; toolMemoOptions: Partial; + toolModules: ToolModule | ToolModule[]; urlRegex: RegExp; version: string; } @@ -68,10 +74,12 @@ interface DefaultOptions { * Overrides for default options. */ type DefaultOptionsOverrides = Partial< - Omit + Omit > & { http?: Partial; logging?: Partial; + pluginIsolation?: 'none' | 'strict' | undefined; + toolModules?: ToolModule | ToolModule[] | undefined; }; /** @@ -129,6 +137,19 @@ interface PluginHostOptions { gracePeriodMs: number; } +/** + * Tools Host options (pure data). Centralized defaults live here. + * + * @property loadTimeoutMs Timeout for child spawn + hello/load/manifest (ms). + * @property invokeTimeoutMs Timeout per external tool invocation (ms). + * @property gracePeriodMs Grace period for external tool invocations (ms). + */ +interface PluginHostOptions { + loadTimeoutMs: number; + invokeTimeoutMs: number; + gracePeriodMs: number; +} + /** * Logging session options, non-configurable by the user. * @@ -315,6 +336,7 @@ const DEFAULT_OPTIONS: DefaultOptions = { logging: LOGGING_OPTIONS, name: packageJson.name, nodeVersion: (process.env.NODE_ENV === 'local' && 22) || getNodeMajorVersion(), + pluginIsolation: 'none', pluginHost: PLUGIN_HOST_OPTIONS, pfExternal: PF_EXTERNAL, pfExternalDesignComponents: PF_EXTERNAL_DESIGN_COMPONENTS, @@ -328,6 +350,7 @@ const DEFAULT_OPTIONS: DefaultOptions = { resourceMemoOptions: RESOURCE_MEMO_OPTIONS, repoName: basename(process.cwd() || '').trim(), toolMemoOptions: TOOL_MEMO_OPTIONS, + toolModules: [], separator: DEFAULT_SEPARATOR, urlRegex: URL_REGEX, version: (process.env.NODE_ENV === 'local' && '0.0.0') || packageJson.version diff --git a/src/server.caching.ts b/src/server.caching.ts index 151755a..267bf42 100644 --- a/src/server.caching.ts +++ b/src/server.caching.ts @@ -60,12 +60,12 @@ type MemoDebugHandler = (info: { type: string; value: unknown * @property {OnMemoCacheHandler} [onCacheExpire] Callback when cache expires. Only fires when the `expire` option is set. * @property {OnMemoCacheHandler} [onCacheRollout] Callback when cache entries are rolled off due to cache limit. */ -interface MemoOptions { +interface MemoOptions { cacheErrors?: boolean; cacheLimit?: number; debug?: MemoDebugHandler; expire?: number; - keyHash?: (args: unknown[]) => unknown; + keyHash?: (args: Readonly, ..._forbidRest: never[]) => unknown; onCacheExpire?: OnMemoCacheHandler; onCacheRollout?: OnMemoCacheHandler; } @@ -98,7 +98,7 @@ const memo = ( keyHash = generateHash, onCacheExpire, onCacheRollout - }: MemoOptions = {} + }: MemoOptions = {} ): (...args: TArgs) => TReturn => { const isCacheErrors = Boolean(cacheErrors); const isFuncPromise = isPromise(func); @@ -107,7 +107,7 @@ const memo = ( const isOnCacheRolloutPromise = isPromise(onCacheRollout); const isOnCacheRollout = typeof onCacheRollout === 'function' || isOnCacheRolloutPromise; const updatedExpire = Number.parseInt(String(expire), 10) || undefined; - const setKey = function (value: unknown[]): unknown { + const setKey = function (value: TArgs): unknown { return keyHash.call(null, value); }; diff --git a/src/server.helpers.ts b/src/server.helpers.ts index d06526a..efd6387 100644 --- a/src/server.helpers.ts +++ b/src/server.helpers.ts @@ -25,6 +25,15 @@ const isPlainObject = (obj: unknown): obj is Record => { return proto === null || proto === Object.prototype; }; +/** + * Is value reference-like? Exclude null and primitives. + * + * @param value + * @returns - `true` if value is reference-like, object or function. + */ +const isReferenceLike = (value: unknown) => + value !== null && (typeof value === 'object' || typeof value === 'function'); + /** * Merge two objects recursively, then return a new object, deep merge. * @@ -275,5 +284,6 @@ export { isObject, isPlainObject, isPromise, + isReferenceLike, mergeObjects }; diff --git a/src/server.tools.ts b/src/server.tools.ts new file mode 100644 index 0000000..42233c7 --- /dev/null +++ b/src/server.tools.ts @@ -0,0 +1,655 @@ +import { spawn, type ChildProcess } from 'node:child_process'; +import { realpathSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname } from 'node:path'; +import { z } from 'zod'; +import { type AppSession, type GlobalOptions } from './options'; +import { type McpToolCreator } from './server'; +import { log, formatUnknownError } from './logger'; +import { + awaitIpc, + send, + makeId, + isHelloAck, + isLoadAck, + isManifestResult, + isInvokeResult, + type ToolDescriptor +} from './server.toolsIpc'; +import { getOptions, getSessionOptions } from './options.context'; +import { setToolOptions } from './options.tools'; +import { normalizeTools, type NormalizedToolEntry } from './server.toolsUser'; +import { jsonSchemaToZod } from './server.schema'; + +/** + * Handle for a spawned Tools Host process. + * + * @property child - Child process + * @property tools - Array of tool descriptors from `tools/list` + * @property closeStderr - Optional function to close stderr reader + */ +type HostHandle = { + child: ChildProcess; + tools: ToolDescriptor[]; + closeStderr?: () => void; +}; + +/** + * Map of active Tools Hosts per session. + */ +const activeHostsBySession = new Map(); + +/** + * Get the tool name from a creator function. + * + * @param creator - Tool creator function + */ +const getBuiltInToolName = (creator: McpToolCreator): string | undefined => + (creator as McpToolCreator & { toolName?: string })?.toolName?.trim?.()?.toLowerCase?.(); + +/** + * Compute the allowlist for the Tools Host. + * + * @param {GlobalOptions} options - Global options. + * @returns Array of absolute directories to allow read access. + */ +const computeFsReadAllowlist = ({ toolModules, contextPath, contextUrl }: GlobalOptions = getOptions()): string[] => { + const directories = new Set(); + const tools = normalizeTools.memo(toolModules, { contextPath, contextUrl }); + + if (contextPath) { + directories.add(contextPath); + } + + tools.forEach(tool => { + if (tool.fsReadDir) { + directories.add(tool.fsReadDir); + } + }); + + return [...directories]; +}; + +/** + * Log warnings and errors from Tools' load. + * + * @param warningsErrors - Object containing warnings and errors + * @param warningsErrors.warnings - Log warnings + * @param warningsErrors.errors - Log errors + */ +const logWarningsErrors = ({ warnings = [], errors = [] }: { warnings?: string[], errors?: string[] } = {}) => { + if (Array.isArray(warnings) && warnings.length > 0) { + const lines = warnings.map(warning => ` - ${String(warning)}`); + + log.warn(`Tools load warnings (${warnings.length})\n${lines.join('\n')}`); + } + + if (Array.isArray(errors) && errors.length > 0) { + const lines = errors.map(error => ` - ${String(error)}`); + + log.warn(`Tools load errors (${errors.length})\n${lines.join('\n')}`); + } +}; + +/** + * Get normalized "file and package" tool modules. + * + * @param {GlobalOptions} options - Global options. + * @param options.contextPath - Base path for tool modules + * @param options.contextUrl - Base URL for tool modules + * @param options.toolModules - Array of tool modules to normalize + * @returns - Filtered array of normalized "file and package" tool modules + */ +const getFilePackageTools = ({ contextPath, contextUrl, toolModules }: GlobalOptions = getOptions()): NormalizedToolEntry[] => + normalizeTools.memo(toolModules, { contextPath, contextUrl }).filter(tool => tool.type === 'file' || tool.type === 'package'); + +/** + * Get normalized "inline" tool modules. + * + * @param {GlobalOptions} options - Global options. + * @param options.contextPath - Base path for tool modules + * @param options.contextUrl - Base URL for tool modules + * @param options.toolModules - Array of tool modules to normalize + * @returns - Filtered array of normalized "inline" tool modules + */ +const getInlineTools = ({ contextPath, contextUrl, toolModules }: GlobalOptions = getOptions()): NormalizedToolEntry[] => + normalizeTools.memo(toolModules, { contextPath, contextUrl }).filter(tool => tool.type === 'tuple' || tool.type === 'object' || tool.type === 'creator'); + +/** + * Get normalized "inline" tool modules. + * + * @param {GlobalOptions} options - Global options. + * @param options.contextPath - Base path for tool modules + * @param options.contextUrl - Base URL for tool modules + * @param options.toolModules - Array of tool modules to normalize + * @returns - Filtered array of normalized "inline" tool modules + */ +const getInvalidTools = ({ contextPath, contextUrl, toolModules }: GlobalOptions = getOptions()): NormalizedToolEntry[] => + normalizeTools.memo(toolModules, { contextPath, contextUrl }).filter(tool => tool.type === 'invalid'); + +/** + * Get normalized file and package tool modules. + * + * @param {GlobalOptions} options - Global options. + * @param options.contextPath - Base path for tool modules + * @param options.contextUrl - Base URL for tool modules + * @param options.toolModules - Array of tool modules to normalize + * @returns Updated array of normalized tool modules + */ +const getFilePackageToolModules = ({ contextPath, contextUrl, toolModules }: GlobalOptions = getOptions()): string[] => + getFilePackageTools({ contextPath, contextUrl, toolModules } as GlobalOptions) + .map(tool => tool.normalizedUrl as string); + +/** + * Debug a child process' stderr output. + * + * @param child - Child process to debug + * @param {AppSession} sessionOptions - Session options + */ +const debugChild = (child: ChildProcess, { sessionId } = getSessionOptions()) => { + const childPid = child.pid; + const promoted = new Set(); + + const debugHandler = (chunk: Buffer | string) => { + const raw = String(chunk); + + if (!raw || !raw.trim()) { + return; + } + + // Split multi-line chunks so each line is tagged + const lines = raw.split(/\r?\n/).map(line => line.trim()).filter(Boolean); + + for (const line of lines) { + const tagged = `[tools-host pid=${childPid} sid=${sessionId}] ${line}`; + + // Pattern: fs read issues + if ( + /ERR_ACCESS_DENIED.*FileSystemRead.*resource:\s*/i.test(line) || + /ERR_ACCESS_DENIED.*Read/i.test(line) + ) { + const key = `fs-deny:${line}`; + + if (!promoted.has(key)) { + promoted.add(key); + log.warn( + `${line}\nTools Host denied fs read. In strict mode, add the resource's directory to --allow-fs-read.\nOptionally, you can disable strict mode entirely with pluginIsolation: 'none'.` + ); + + continue; + } + } + + // Pattern: ESM/CJS import issues + if ( + /ERR_MODULE_NOT_FOUND/.test(line) || + /Cannot use import statement outside a module/i.test(line) || + /ERR_UNKNOWN_FILE_EXTENSION/.test(line) + ) { + const key = `esm:${line}`; + + if (!promoted.has(key)) { + promoted.add(key); + log.warn('Tools Host import error. Ensure external tools are ESM (no raw .ts) and resolvable.\nFor local files, prefer a file:// URL.'); + + continue; + } + } + + // Default: debug-level passthrough + log.debug(tagged); + } + }; + + child.stderr?.on?.('data', debugHandler); + + return () => { + child.stderr?.off?.('data', debugHandler); + }; +}; + +/** + * Spawn the Tools Host (child process), load external tools, and return a host handle. + * + * - See `package.json` import path for entry parameter. + * - Requires Node ≥ 22 for process isolation flags. + * - Attaches a stderr reader for debugging if protocol logging is enabled. + * - Returns descriptors from `tools/list` and an IPC-capable child. + * + * @param {GlobalOptions} options - Global options. + * @returns Host handle used by `makeProxyCreators` and shutdown. + */ +const spawnToolsHost = async ( + options: GlobalOptions = getOptions() +): Promise => { + const { nodeVersion, pluginIsolation, pluginHost } = options || {}; + const { loadTimeoutMs, invokeTimeoutMs } = pluginHost || {}; + const nodeArgs: string[] = []; + let updatedEntry: string | undefined = undefined; + + try { + const entryUrl = import.meta.resolve('#toolsHost'); + + updatedEntry = fileURLToPath(entryUrl); + } catch (error) { + log.debug(`Failed to import.meta.resolve Tools Host entry '#toolsHost': ${formatUnknownError(error)}`); + + if (process.env.NODE_ENV === 'local') { + updatedEntry = '/mock/path/to/toolsHost.js'; + } + } + + if (updatedEntry === undefined) { + throw new Error(`Failed to resolve Tools Host entry '#toolsHost'.`); + } + + // Deny network and fs write by omission + if (pluginIsolation === 'strict') { + // Node 24+ moves to using the "--permission" flag instead of "--experimental-permission" + const permissionFlag = nodeVersion >= 24 ? '--permission' : '--experimental-permission'; + + nodeArgs.push(permissionFlag); + + // 1) Gather directories (project, plugin modules, and the host entry's dir) + const allowSet = new Set(computeFsReadAllowlist()); + + allowSet.add(dirname(updatedEntry)); + + // 2) Normalize to real absolute paths to avoid symlink mismatches + // Using top-level import instead of dynamic import for better performance + const allowList = [...allowSet] + .map(dir => { + try { + return realpathSync(dir); + } catch { + return dir; + } + }) + .filter(Boolean); + + // 3) Pass one --allow-fs-read per directory (more robust than a single comma-separated flag) + for (const dir of allowList) { + nodeArgs.push(`--allow-fs-read=${dir}`); + } + + // Optional debug to verify exactly what the child gets + log.debug(`Tools Host allow-fs-read flags: ${allowList.map(dir => `--allow-fs-read=${dir}`).join(' ')}`); + log.debug(`Tools Host permission flag: ${permissionFlag}`); + } + + // Pre-compute file and package tool modules before spawning to reduce latency + const filePackageToolModules = getFilePackageToolModules(); + + const child: ChildProcess = spawn(process.execPath, [...nodeArgs, updatedEntry], { + stdio: ['ignore', 'pipe', 'pipe', 'ipc'] + }); + + const closeStderr = debugChild(child); + + // hello + send(child, { t: 'hello', id: makeId() }); + await awaitIpc(child, isHelloAck, loadTimeoutMs); + + // load + const loadId = makeId(); + + // Pass a focused set of tool options to the host. Avoid the full options object. + const toolOptions = setToolOptions(options); + + send(child, { t: 'load', id: loadId, specs: filePackageToolModules, invokeTimeoutMs, toolOptions }); + const loadAck = await awaitIpc(child, isLoadAck(loadId), loadTimeoutMs); + + logWarningsErrors(loadAck); + + // manifest + const manifestRequestId = makeId(); + + send(child, { t: 'manifest:get', id: manifestRequestId }); + const manifest = await awaitIpc(child, isManifestResult(manifestRequestId), loadTimeoutMs); + + return { child, tools: manifest.tools as ToolDescriptor[], closeStderr }; +}; + +/** + * Recreate parent-side tool creators that forward invocations to the Tools Host. + * - Parent does not perform validation; the child validates with Zod at invocation. + * - A minimal Zod inputSchema from the parent is required to trigger the MCP SDK parameter + * validation. + * - There is an unreachable defensive check in `makeProxyCreators` that ensures the Zod schema + * always returns a value. + * - Invocation errors from the child preserve `error.code` and `error.details` for debugging. + * + * @param {HostHandle} handle - Tools Host handle. + * @param {GlobalOptions} options - Global options. + * @returns Array of tool creators + */ +const makeProxyCreators = ( + handle: HostHandle, + { pluginHost }: GlobalOptions = getOptions() +): McpToolCreator[] => handle.tools.map((tool): McpToolCreator => () => { + const name = tool.name; + const invokeTimeoutMs = Math.max(0, Number(pluginHost?.invokeTimeoutMs) || 0); + + // Rebuild Zod schema from serialized JSON. + const zodSchemaStrict = jsonSchemaToZod(tool.inputSchema); + let zodSchema = zodSchemaStrict; + + // Rebuild Zod schema again for compatibility. + if (!zodSchemaStrict) { + zodSchema = jsonSchemaToZod(tool.inputSchema, { failFast: false }); + + log.debug( + `Tool "${name}" from ${tool.source || 'unknown source'} failed strict JSON to Zod reconstruction.`, + `Using fallback best-effort schema. Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.`, + `[ZOD_SCHEMA: defined: ${Boolean(zodSchema)}]` + ); + } + + // Defensive check only. Currently, unreachable due to `jsonSchemaToZod`'s current return/response. Zod is integral + // to the MCP SDK, in the unlikely event that the Zod schema is still unavailable, fallback again. All hail Zod! + if (!zodSchema) { + zodSchema = z.looseObject({}); + + log.error( + `Tool "${name}" from ${tool.source || 'unknown source'} failed strict and best‑effort JSON to Zod reconstruction.`, + `Falling back to permissive schema for SDK broadcast. Review the inputSchema.`, + `[ZOD_SCHEMA: defined: ${Boolean(zodSchema)}]` + ); + } + + // Broadcast the tool's input schema towards clients/agents. + const schema = { + description: tool.description, + inputSchema: zodSchema + }; + + const handler = async (args: unknown) => { + const requestId = makeId(); + + send(handle.child, { t: 'invoke', id: requestId, toolId: tool.id, args }); + + const response = await awaitIpc( + handle.child, + isInvokeResult(requestId), + invokeTimeoutMs + ); + + if ('ok' in response && response.ok === false) { + const invocationError = new Error(response.error?.message || 'Tool invocation failed', { cause: response.error?.cause }) as Error & { + code?: string; + details?: unknown; + }; + + if (response.error?.stack) { + invocationError.stack = response.error.stack; + } + + if (response.error?.code) { + invocationError.code = response.error?.code; + } + + invocationError.details = response.error?.details || (response as any).error?.cause?.details; + throw invocationError; + } + + return response.result; + }; + + return [name, schema, handler]; +}); + +/** + * Best-effort Tools Host shutdown for the current session. + * + * Policy: + * - Primary grace defaults to 0 ms (internal-only, from DEFAULT_OPTIONS.pluginHost.gracePeriodMs) + * - Single fallback kill at grace + 200 ms to avoid racing simultaneous kills + * - Close logging for child(ren) stderr + * + * @param {GlobalOptions} options - Global options. + * @param {AppSession} sessionOptions - Session options. + */ +const sendToolsHostShutdown = async ( + { pluginHost }: GlobalOptions = getOptions(), + { sessionId }: AppSession = getSessionOptions() +): Promise => { + const handle = activeHostsBySession.get(sessionId); + + if (!handle) { + return; + } + + const gracePeriodMs = Math.max(0, Number(pluginHost?.gracePeriodMs) || 0); + const fallbackGracePeriodMs = gracePeriodMs + 200; + + const child = handle.child; + let resolved = false; + let forceKillPrimary: NodeJS.Timeout | undefined; + let forceKillSecondary: NodeJS.Timeout | undefined; + let resolveIt: ((value: PromiseLike | void) => void) | undefined; + + // Attempt exit, disconnect, then remove from activeHostsBySession and finally resolve + const shutdownChild = () => { + if (resolved) { + return; + } + + resolved = true; + child.off('exit', shutdownChild); + child.off('disconnect', shutdownChild); + + if (forceKillPrimary) { + clearTimeout(forceKillPrimary); + } + + if (forceKillSecondary) { + clearTimeout(forceKillSecondary); + } + + try { + (handle as any).closeStderr(); + log.info('Tools Host stderr reader closed.'); + } catch (error) { + log.error(`Failed to close Tools Host stderr reader: ${formatUnknownError(error)}`); + } + + const confirmHandle = activeHostsBySession.get(sessionId); + + if (confirmHandle?.child === child) { + activeHostsBySession.delete(sessionId); + } + + resolveIt?.(); + }; + + // Forced shutdown. + const sigkillChild = (isSecondaryFallback: boolean = false) => { + try { + if (!child?.killed) { + log.warn( + `${ + (resolved && 'Already attempted shutdown.') || 'Slow shutdown response.' + } ${ + (isSecondaryFallback && 'Secondary') || 'Primary' + } fallback force-killing Tools Host child process.` + ); + child.kill('SIGKILL'); + } + } catch (error) { + log.error(`Failed to force-kill Tools Host child process: ${formatUnknownError(error)}`); + } + }; + + // Start the shutdown process + await new Promise(resolve => { + resolveIt = resolve; + // Send a shutdown signal to child. We try/catch in case the process is already dead, and + // since we're still following it up with a graceful shutdown, then force-kill. + try { + send(child, { t: 'shutdown', id: makeId() }); + } catch (error) { + log.error(`Failed to send shutdown signal to Tools Host child process: ${formatUnknownError(error)}`); + } + + // Set primary timeout for force shutdown + forceKillPrimary = setTimeout(() => { + sigkillChild(); + shutdownChild(); + }, gracePeriodMs); + forceKillPrimary?.unref?.(); + + // Set fallback timeout for force shutdown + forceKillSecondary = setTimeout(() => { + sigkillChild(true); + shutdownChild(); + }, fallbackGracePeriodMs); + forceKillSecondary?.unref?.(); + + // Set up exit/disconnect handlers to resolve + child.once('exit', shutdownChild); + child.once('disconnect', shutdownChild); + }); +}; + +/** + * Compose built-in creators with any externally loaded creators. + * + * - Node.js version policy: + * - Node >= 22, external plugins are executed out-of-process via a Tools Host. + * - Node < 22, externals are skipped with a warning and only built-ins are returned. + * - Registry is self‑correcting for pre‑load or mid‑run crashes without changing normal shutdown + * + * @param builtinCreators - Built-in tool creators + * @param {GlobalOptions} options - Global options. + * @param {AppSession} sessionOptions - Session options. + * @returns {Promise} Promise array of tool creators + */ +const composeTools = async ( + builtinCreators: McpToolCreator[], + { toolModules, nodeVersion, contextUrl, contextPath }: GlobalOptions = getOptions(), + { sessionId }: AppSession = getSessionOptions() +): Promise => { + const toolCreators: McpToolCreator[] = [...builtinCreators]; + const usedNames = new Set(builtinCreators.map(creator => getBuiltInToolName(creator)).filter(Boolean) as string[]); + + if (!Array.isArray(toolModules) || toolModules.length === 0) { + log.info('No external tools loaded.'); + + return toolCreators; + } + + const filePackageCreators: NormalizedToolEntry[] = getFilePackageTools({ toolModules, contextUrl, contextPath } as GlobalOptions); + const invalidCreators = getInvalidTools({ toolModules, contextUrl, contextPath } as GlobalOptions); + const inlineCreators: NormalizedToolEntry[] = getInlineTools({ toolModules, contextUrl, contextPath } as GlobalOptions); + + const normalizeToolName = (toolName?: string) => toolName?.trim?.()?.toLowerCase?.(); + + invalidCreators.forEach(({ error }) => { + log.warn(error); + }); + + const filteredInlineCreators: McpToolCreator[] = inlineCreators.map(tool => { + const toolName = normalizeToolName(tool.toolName); + + if (toolName && usedNames.has(toolName)) { + log.warn(`Skipping inline tool "${toolName}" because a tool with the same name is already provided (built-in or earlier).`); + + return undefined; + } + + if (toolName) { + usedNames.add(toolName); + } + + return tool.value as McpToolCreator; + }).filter(Boolean) as McpToolCreator[]; + + toolCreators.push(...filteredInlineCreators); + + // Load file-based via Tools Host (Node.js version gate applies here) + if (filePackageCreators.length === 0) { + return toolCreators; + } + + if (!nodeVersion || nodeVersion < 22) { + log.warn('External tool plugins require Node >= 22; skipping file-based tools.'); + + return toolCreators; + } + + let host: HostHandle | undefined; + + // Clean up on exit or disconnect + const onChildExitOrDisconnect = () => { + if (!host) { + return; + } + + const current = activeHostsBySession.get(sessionId); + + if (current && current.child === host.child) { + try { + (host as any).closeStderr(); + log.info('Tools Host stderr reader closed.'); + } catch (error) { + log.error(`Failed to close Tools Host stderr reader: ${formatUnknownError(error)}`); + } + + activeHostsBySession.delete(sessionId); + } + + host.child.off('exit', onChildExitOrDisconnect); + host.child.off('disconnect', onChildExitOrDisconnect); + }; + + try { + host = await spawnToolsHost(); + + // Filter manifest by reserved names BEFORE proxying + const filteredTools = host.tools.filter(tool => { + const toolName = normalizeToolName(tool.name); + + if (toolName && usedNames.has(toolName)) { + log.warn(`Skipping plugin tool "${tool.name}" – name already used by built-in/inline tool.`); + + return false; + } + + if (toolName) { + usedNames.add(toolName); + } + + return true; + }); + + const filteredHandle = { ...host, tools: filteredTools } as HostHandle; + const proxiedCreators = makeProxyCreators(filteredHandle); + + // Associate the spawned host with the current session + activeHostsBySession.set(sessionId, host); + + host.child.once('exit', onChildExitOrDisconnect); + host.child.once('disconnect', onChildExitOrDisconnect); + + return [...toolCreators, ...proxiedCreators]; + } catch (error) { + log.warn(`Failed to start Tools Host; skipping externals and continuing with built-ins/inline. ${formatUnknownError(error)}`); + + return toolCreators; + } +}; + +export { + composeTools, + computeFsReadAllowlist, + debugChild, + getBuiltInToolName, + getFilePackageTools, + getInlineTools, + getInvalidTools, + getFilePackageToolModules, + logWarningsErrors, + makeProxyCreators, + sendToolsHostShutdown, + spawnToolsHost +}; diff --git a/src/server.toolsUser.ts b/src/server.toolsUser.ts new file mode 100644 index 0000000..23bbd48 --- /dev/null +++ b/src/server.toolsUser.ts @@ -0,0 +1,759 @@ +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { dirname, extname, isAbsolute, resolve } from 'node:path'; +import { isPlainObject, isReferenceLike } from './server.helpers'; +import { type McpToolCreator, type McpTool } from './server'; +import { type GlobalOptions } from './options'; +import { memo } from './server.caching'; +import { DEFAULT_OPTIONS } from './options.defaults'; +import { formatUnknownError } from './logger'; +import { normalizeInputSchema } from './server.schema'; + +/** + * A normalized tool entry for normalizing values for strings and tool creators. + * + * @property type - Classification of the entry (file, package, creator, tuple, object, invalid) + * @property index - The original input index (for diagnostics) + * @property original - The original input value + * @property value - The final consumer value (string or creator) + * @property toolName - The tool name for tuple/object/function entries + * @property normalizedUrl - The normalized file URL for file entries + * @property fsReadDir - The directory to include in allowlist for file, or package, entries + * @property isUrlLike - File, or package, URL indicator + * @property isFilePath - File, or package, path indicator + * @property isFileUrl - File, or package, URL indicator + * @property error - Error message for invalid entries + */ +type NormalizedToolEntry = { + type: 'file' | 'package' | 'creator' | 'tuple' | 'object' | 'invalid'; + index: number; + original: unknown; + value: string | McpToolCreator; + toolName?: string; + normalizedUrl?: string; + fsReadDir?: string | undefined; + isUrlLike?: boolean; + isFilePath?: boolean; + isFileUrl?: boolean; + error?: string | undefined; +}; + +/** + * A file or package tool entry for normalizing values for strings. + */ +type FileEntry = Pick; + +/** + * A general tool entry for normalizing values for creators. + */ +type CreatorEntry = Pick; + +/** + * An MCP tool "wrapper", or "creator". + * + * @alias McpToolCreator + */ +type ToolCreator = McpToolCreator; + +/** + * An MCP tool. Standalone or returned by `createMcpTool`. + * + * @alias McpTool + */ +type Tool = McpTool; + +/** + * Author-facing "tools as plugins" surface. + * + * A tool module is a flexible type that supports either a single string identifier, + * a specific tool creator, or multiple tool creators. + * + * - A `file path` or `file URL` string, that refers to the name or identifier of a local ESM tool package. + * - A `package name` string, that refers to the name or identifier of a local ESM tool package. + * - An `McpTool`, a tuple of `[toolName, toolConfig, toolHandler]` + * - An `McpToolCreator`, a function that returns an `McpTool`. + * - An array of `McpToolCreator` functions. + */ +type ToolModule = (string | McpTool | McpToolCreator | McpToolCreator[])[] | string | McpTool | McpToolCreator | McpToolCreator[]; + +// type ToolModule = string | McpTool | McpToolCreator | (string | McpTool | McpToolCreator)[]; +// type ToolModules = string | McpTool | McpToolCreator | McpToolCreator[]; + +/** + * Author-facing tool config. The handler may be async or sync. + * + * @template TArgs The type of arguments expected by the tool (optional). + * @template TResult The type of result returned by the tool (optional). + * + * @property name - Name of the tool + * @property description - Description of the tool + * @property inputSchema - JSON Schema or Zod schema describing the arguments expected by the tool + * @property {(args: TArgs, options?: GlobalOptions) => Promise | TResult} handler - Tool handler + * - `args` are returned by the tool's `inputSchema`' + * - `options` are currently unused and reserved for future use. + */ +type ToolConfig = { + name: string; + description: string; + inputSchema: unknown; + handler: (args: TArgs, options?: GlobalOptions) => Promise | TResult; +}; + +/** + * Author-facing tool schema. + * + * @property description - Description of the tool + * @property inputSchema - JSON Schema or Zod schema describing the arguments expected by the tool + */ +type ToolSchema = { + inputSchema: unknown; + description: string; +}; + +/** + * Author-facing multi-tool config. + * + * @property [name] - Optional name for the group of tools + * @property {ToolConfig} tools - Array of tool configs + */ +type MultiToolConfig = { + name?: string | undefined; + tools: ToolConfig[] +}; + +/** + * Allowed keys in the tool config objects. Expand as needed. + */ +const ALLOWED_CONFIG_KEYS = new Set(['name', 'description', 'inputSchema', 'handler']); + +/** + * Allowed keys in the tool schema objects. Expand as needed. See related `ToolSchema`. + */ +const ALLOWED_SCHEMA_KEYS = new Set(['description', 'inputSchema']); + +/** + * Memoization key store. See `getSetMemoKey`. + */ +const toolsMemoKeyStore: WeakMap> = new WeakMap(); + +/** + * Quick consistent unique key, via symbol (anything unique-like will work), for a given input + * and context. + * + * Used specifically for helping memoize functions and objects against context. Not used + * elsewhere because simple equality checks, without context, in the lower-level functions + * are good enough. + * + * @private + * @param input - Input can be an object, function, or primitive value. + * @param contextKey - Additional context to help uniqueness. + * @returns A unique key, a symbol for objects/functions or string for primitives. + */ +const getSetMemoKey = (input: unknown, contextKey: string) => { + if (!isReferenceLike(input)) { + return `${String(input)}:${contextKey}`; + } + + let contextMap = toolsMemoKeyStore.get(input); + let token; + + if (!contextMap) { + contextMap = new Map(); + toolsMemoKeyStore.set(input, contextMap); + } + + token = contextMap.get(contextKey); + + if (!token) { + token = Symbol(`tools:${contextKey}`); + contextMap.set(contextKey, token); + } + + return token; +}; + +/** + * Return an object key value. + * + * @param obj + * @param key + */ +const sanitizeDataProp = (obj: unknown, key: string) => { + if (!isReferenceLike(obj)) { + return undefined; + } + + const descriptor = Object.getOwnPropertyDescriptor(obj, key); + const isDataProp = descriptor !== undefined && 'value' in descriptor; + + if (isDataProp && typeof descriptor?.get !== 'function' && typeof descriptor?.set !== 'function') { + return descriptor; + } + + return undefined; +}; + +/** + * Sanitize a plain object for allowed keys. + * + * @param obj + * @param allowedKeys + */ +const sanitizePlainObject = (obj: unknown, allowedKeys: Set) => { + const updatedObj = {} as Record; + + if (!isPlainObject(obj)) { + return updatedObj; + } + + for (const key of Object.keys(obj as object)) { + if (!allowedKeys.has(key)) { + continue; + } + + const prop = sanitizeDataProp(obj, key); + + if (prop === undefined) { + continue; + } + + updatedObj[key] = prop?.value; + } + + return updatedObj; +}; + +/** + * Check if a string looks like a file path. + * + * @param str + * @returns Confirmation that the string looks like a file path. + */ +const isFilePath = (str: string): boolean => { + if (typeof str !== 'string') { + return false; + } + + return str.startsWith('./') || str.startsWith('../') || str.startsWith('/') || /^[A-Za-z]:[\\/]/.test(str) || extname(str).length >= 2; +}; + +/** + * Check if a string looks like a URL. + * + * @param str + * @returns Confirmation that the string looks like a URL. + */ +const isUrlLike = (str: string) => + /^(file:|https?:|data:|node:)/i.test(str); + +/** + * Normalize a tuple object with schema into a Zod schema. + * + * @param schema + * @param allowedKeys + */ +const normalizeTupleSchema = (schema: unknown, allowedKeys = ALLOWED_SCHEMA_KEYS) => { + if (!isPlainObject(schema)) { + return undefined; + } + + const { description, inputSchema } = sanitizePlainObject(schema, allowedKeys); + + const updatedDesc = (description as string)?.trim?.() || undefined; + const updatedSchema = normalizeInputSchema(inputSchema); + + if (!updatedSchema) { + return undefined; + } + + const obj: { inputSchema: unknown, description?: string } = { inputSchema: updatedSchema }; + + if (updatedDesc) { + obj.description = updatedDesc as string; + } + + return obj; +}; + +/** + * Memoize the `normalizeSchema` function. + */ +normalizeTupleSchema.memo = memo(normalizeTupleSchema, { cacheErrors: false, keyHash: args => args[0] }); + +/** + * Normalize a tuple config into a tool creator function. + * + * @param config - The array configuration to normalize. + * @returns A tool creator function, or undefined if the config is invalid. + */ +const normalizeTuple = (config: unknown): CreatorEntry | undefined => { + if (!Array.isArray(config) || config.length !== 3) { + return undefined; + } + + const name = sanitizeDataProp(config, '0'); + const schema = sanitizeDataProp(config, '1'); + const handler = sanitizeDataProp(config, '2'); + + if (!name || !schema || !handler) { + return undefined; + } + + const updatedName = (name.value as string)?.trim?.() || undefined; + const updatedSchema = normalizeTupleSchema.memo(schema.value); + const updatedHandler = typeof handler.value === 'function' ? handler.value : undefined; + + if (!updatedName || !updatedHandler) { + return undefined; + } + + const creator: ToolCreator = () => [ + updatedName as string, + updatedSchema as ToolSchema, + updatedHandler as (args: unknown) => unknown | Promise + ]; + + (creator as any).toolName = updatedName as string; + + let err: string | undefined; + + if (!updatedSchema) { + err = `Tool "${updatedName}" failed to set inputSchema. Provide a Zod schema, a Zod raw shape, or a plain JSON Schema object.`; + } + + return { + original: config, + toolName: updatedName as string, + type: err ? 'invalid' : 'tuple', + value: creator, + ...(err ? { error: err } : {}) + }; +}; + +/** + * Memoize the `normalizeTuple` function. + */ +normalizeTuple.memo = memo(normalizeTuple, { cacheErrors: false, keyHash: args => args[0] }); + +/** + * Normalize an object config into a tool creator function. + * + * @param config - The object configuration to normalize. + * @param allowedKeys - Allowed keys in the config object. + * @returns A tool creator function, or undefined if the config is invalid. + */ +const normalizeObject = (config: unknown, allowedKeys = ALLOWED_CONFIG_KEYS): CreatorEntry | undefined => { + if (!isPlainObject(config)) { + return undefined; + } + + const { name, description, inputSchema, handler } = sanitizePlainObject(config, allowedKeys); + + const updatedName = (name as string)?.trim?.() || undefined; + const updatedDesc = (description as string)?.trim?.() || undefined; + const updatedSchema = normalizeInputSchema(inputSchema); + const updatedHandler = typeof handler === 'function' ? handler : undefined; + + if (!updatedName || !updatedDesc || !updatedHandler) { + return undefined; + } + + const creator: ToolCreator = () => [ + updatedName as string, + { + description: updatedDesc as string, + inputSchema: updatedSchema + }, + updatedHandler as (args: unknown) => unknown | Promise + ]; + + (creator as any).toolName = updatedName as string; + + let err: string | undefined; + + if (!updatedSchema) { + err = `Tool "${updatedName}" failed to set inputSchema. Provide a Zod schema, a Zod raw shape, or a plain JSON Schema object.`; + } + + return { + original: config, + toolName: updatedName as string, + type: err ? 'invalid' : 'object', + value: creator, + ...(err ? { error: err } : {}) + }; +}; + +/** + * Memoize the `normalizeObject` function. + */ +normalizeObject.memo = memo(normalizeObject, { cacheErrors: false, keyHash: args => args[0] }); + +/** + * Normalize a creator function into a tool creator function. + * + * @param config + * @returns A tool creator function, or undefined if the config is invalid. + */ +const normalizeFunction = (config: unknown): CreatorEntry | undefined => { + if (typeof config !== 'function') { + return undefined; + } + + const originalConfig = config as ToolCreator; + + const wrappedConfig: ToolCreator = (opts?: unknown) => { + let response; + + try { + response = originalConfig.call(null, opts as unknown as GlobalOptions); + } catch (error) { + throw new Error(`Tool failed to load: ${formatUnknownError(error)}`); + } + + // Currently, we only support tuples in creator functions. + if (normalizeTuple.memo(response)) { + const { value } = normalizeTuple.memo(response) || {}; + + return (value as ToolCreator)?.(); + } + + return response; + }; + + (wrappedConfig as any).toolName = (config as any).toolName; + + return { + original: config, + toolName: (config as any).toolName, + type: 'creator', + value: wrappedConfig as ToolCreator + }; +}; + +/** + * Memoize the `normalizeFunction` function. + */ +normalizeFunction.memo = memo(normalizeFunction, { cacheErrors: false, keyHash: args => args[0] }); + +/** + * Normalize a file URL into a file entry. + * + * @param config - The file URL to normalize. + * @returns - A file entry, or undefined if the config is invalid. + */ +const normalizeFileUrl = (config: unknown): FileEntry | undefined => { + if (typeof config !== 'string' || !config.startsWith('file:')) { + return undefined; + } + + const entry: Partial = { isUrlLike: isUrlLike(config), isFilePath: isFilePath(config) }; + const err: string[] = []; + const isFileUrl = config.startsWith('file:'); + const normalizedUrl = config; + let fsReadDir: string | undefined = undefined; + let type: NormalizedToolEntry['type'] = 'invalid'; + + try { + const resolvedPath = fileURLToPath(config); + + fsReadDir = dirname(resolvedPath); + type = 'file'; + } catch (error) { + err.push(`Failed to resolve file url: ${config}: ${formatUnknownError(error)}`); + } + + return { + ...entry, + normalizedUrl, + fsReadDir, + isFileUrl, + original: config, + type, + value: config, + ...(type === 'invalid' ? { error: err.join('\n') } : {}) + }; +}; + +/** + * Memoize the `normalizeFileUrl` function. + */ +normalizeFileUrl.memo = memo(normalizeFileUrl, { cacheErrors: false, keyHash: args => args[0] }); + +/** + * Normalize a file path into a file entry. + * + * File URLs are handled by `normalizeFileUrl`. + * + * @param config - The file path to normalize. + * @param options - Optional settings + * @param options.contextPath - The context path to use for resolving file paths. + * @param options.contextUrl - The context URL to use for resolving file paths. + * @returns - A file entry, or undefined if the config is invalid. + */ +const normalizeFilePath = ( + config: unknown, + { + contextPath = DEFAULT_OPTIONS.contextPath, + contextUrl + }: { contextPath?: string, contextUrl?: string } = {} +): FileEntry | undefined => { + if (typeof config !== 'string' || !isFilePath(config) || isUrlLike(config)) { + return undefined; + } + + const entry: Partial = { isUrlLike: isUrlLike(config), isFilePath: isFilePath(config) }; + const err: string[] = []; + let isFileUrl = config.startsWith('file:'); + let normalizedUrl = config; + let fsReadDir: string | undefined = undefined; + let type: NormalizedToolEntry['type'] = 'invalid'; + + try { + if (contextUrl !== undefined) { + const url = import.meta.resolve(config, contextUrl); + + if (url.startsWith('file:')) { + const resolvedPath = fileURLToPath(url); + + fsReadDir = dirname(resolvedPath); + normalizedUrl = pathToFileURL(resolvedPath).href; + isFileUrl = true; + type = 'file'; + } + } + + // Fallback if resolve() path failed or not file: + if (type !== 'file') { + const resolvedPath = isAbsolute(config) ? config : resolve(contextPath as string, config); + + fsReadDir = dirname(resolvedPath as string); + normalizedUrl = pathToFileURL(resolvedPath as string).href; + isFileUrl = true; + type = 'file'; + } + } catch (error) { + err.push(`Failed to resolve file path: ${config}: ${formatUnknownError(error)}`); + } + + return { + ...entry, + normalizedUrl, + fsReadDir, + isFileUrl, + original: config, + type, + value: config, + ...(type === 'invalid' ? { error: err.join('\n') } : {}) + }; +}; + +/** + * Memoize the `normalizeFilePath` function. + */ +normalizeFilePath.memo = memo(normalizeFilePath, { + cacheErrors: false, + keyHash: args => + JSON.stringify([args[0], (args as any)?.[1]?.contextPath, (args as any)?.[1]?.contextUrl]) +}); + +/** + * Normalize a file or package tool config into a file entry. + * + * - First checks if the config is a file URL. If so, derive fsReadDir for allow-listing. + * - Next, checks if the config looks like a filesystem path. If so, resolve. + * - Otherwise, keep as-is (package name or other URL-like spec). + * + * @param config - The file, or package, configuration to normalize. + * @param options - Optional settings + * @param options.contextPath - The context path to use for resolving file paths. + * @param options.contextUrl - The context URL to use for resolving file paths. + * @returns {FileEntry} + */ +const normalizeFilePackage = ( + config: unknown, + { contextPath, contextUrl }: { contextPath?: string, contextUrl?: string } = {} +): FileEntry | undefined => { + if (typeof config !== 'string') { + return undefined; + } + + // Case 1: already a file URL -> derive fsReadDir for allow-listing + if (normalizeFileUrl.memo(config)) { + return normalizeFileUrl.memo(config); + } + + // Case 2: looks like a filesystem path -> resolve or invalid + if (normalizeFilePath.memo(config, { contextPath, contextUrl } as any)) { + return normalizeFilePath.memo(config, { contextPath, contextUrl } as any); + } + + // Case 3: non-file string -> keep as-is (package name or other URL-like spec) + // Note: http(s) module specs are not supported by Node import and will surface as load warnings in the child. + return { + isUrlLike: isUrlLike(config), + isFilePath: isFilePath(config), + normalizedUrl: config, + fsReadDir: undefined, + isFileUrl: false, + original: config, + type: 'package', + value: config + }; +}; + +/** + * Memoize the `normalizeFilePackage` function. + */ +normalizeFilePackage.memo = memo(normalizeFilePackage, { + cacheErrors: false, + keyHash: args => + JSON.stringify([args[0], (args as any)?.[1]?.contextPath, (args as any)?.[1]?.contextUrl]) +}); + +/** + * Normalize tool configuration(s) into a normalized tool entry. + * + * @param config - The configuration(s) to normalize. + * @param options - Optional settings + * @param options.contextPath - The context path to use for resolving file paths. + * @param options.contextUrl - The context URL to use for resolving file paths. + * @returns An array of normalized tool entries. + */ +const normalizeTools = (config: any, { + contextPath = DEFAULT_OPTIONS.contextPath, + contextUrl = DEFAULT_OPTIONS.contextUrl +}: { contextPath?: string, contextUrl?: string } = {}): NormalizedToolEntry[] => { + // const updatedConfigs = (normalizeTuple.memo(config) && [config]) || (Array.isArray(config) && config) || (config && [config]) || []; + const updatedConfigs = (normalizeTuple.memo(config) && [config]) || (Array.isArray(config) && config) || [config]; + const normalizedConfigs: NormalizedToolEntry[] = []; + + // Flatten nested-arrays of configs and attempt to account for inline tuples. This will catch + // one-off cases where an array will be flattened, broken apart. + const flattenedConfigs = updatedConfigs.flatMap((item: unknown) => + (normalizeTuple.memo(item) && [item]) || (Array.isArray(item) && item) || [item]); + // (normalizeTuple.memo(item) && [item]) || (Array.isArray(item) && item) || (item && [item]) || []; + + flattenedConfigs.forEach((config: unknown, index: number) => { + if (normalizeFunction.memo(config)) { + normalizedConfigs.push({ + index, + ...normalizeFunction.memo(config) as CreatorEntry + }); + + return; + } + + if (normalizeTuple.memo(config)) { + normalizedConfigs.push({ + index, + ...normalizeTuple.memo(config) as CreatorEntry + }); + + return; + } + + if (normalizeObject.memo(config)) { + normalizedConfigs.push({ + index, + ...normalizeObject.memo(config) as CreatorEntry + }); + + return; + } + + if (normalizeFilePackage.memo(config, { contextPath, contextUrl })) { + normalizedConfigs.push({ + index, + ...normalizeFilePackage.memo(config, { contextPath, contextUrl }) as FileEntry + }); + + return; + } + + const err = `createMcpTool: invalid configuration used at index ${index}: Unsupported type ${typeof config}`; + + normalizedConfigs.push({ + index, + original: config, + type: 'invalid', + value: err, + error: err + }); + }); + + return normalizedConfigs; +}; + +/** + * Memoize the `normalizeTools` function. + */ +normalizeTools.memo = memo(normalizeTools, { + cacheErrors: false, + keyHash: args => + getSetMemoKey(args[0], `${(args as any)?.[1]?.contextPath}:${(args as any)?.[1]?.contextUrl}`) +}); + +/** + * Author-facing helper for creating an MCP tool configuration list for Patternfly MCP server. + * + * @example A single file path string + * export default createMcpTool('./a/file/path.mjs'); + * + * @example A single package string + * export default createMcpTool('@my-org/my-tool'); + * + * @example A single tool configuration tuple + * export default createMcpTool(['myTool', { description: 'My tool description' }, (args) => { ... }]); + * + * @example A single tool creator function + * export default createMcpTool(() => ['myTool', { description: 'My tool description' }, (args) => { ... }]); + * + * @example A single tool configuration object + * export default createMcpTool({ name: 'myTool', description: 'My tool description', inputSchema: {}, handler: (args) => { ... } }); + * + * @example A multi-tool configuration array/list + * export default createMcpTool(['./a/file/path.mjs', { name: 'myTool', description: 'My tool description', inputSchema: {}, handler: (args) => { ... } }]); + * + * @param config - The configuration for creating the tool(s). It can be: + * - A single string representing the name of a local ESM predefined tool (`file path string` or `file URL string`). Limited to Node.js 22+ + * - A single string representing the name of a local ESM tool package (`package string`). Limited to Node.js 22+ + * - A single inline tool configuration tuple (`Tool`). + * - A single inline tool creator function returning a tuple (`ToolCreator`). + * - A single inline tool configuration object (`ToolConfig`). + * - An array of the aforementioned configuration types in any combination. + * @returns An array of strings and/or tool creators that can be applied to the MCP server `toolModules` option. + * + * @throws {Error} If a configuration is invalid, an error is thrown on the first invalid entry. + */ +const createMcpTool = (config: unknown): ToolModule => { + const entries = normalizeTools.memo(config); + const err = entries.find(entry => entry.type === 'invalid'); + + if (err?.error) { + throw new Error(err.error); + } + + return entries.map(entry => entry.value); +}; + +export { + createMcpTool, + isFilePath, + isUrlLike, + normalizeFilePackage, + normalizeFileUrl, + normalizeFilePath, + normalizeTuple, + normalizeTupleSchema, + normalizeObject, + normalizeFunction, + normalizeTools, + sanitizeDataProp, + sanitizePlainObject, + type MultiToolConfig, + type NormalizedToolEntry, + type ToolCreator, + type Tool, + type ToolConfig, + type ToolModule +}; diff --git a/src/tool.componentSchemas.ts b/src/tool.componentSchemas.ts index 0fb2b2b..0186cc2 100644 --- a/src/tool.componentSchemas.ts +++ b/src/tool.componentSchemas.ts @@ -91,4 +91,9 @@ const componentSchemasTool = (options = getOptions()): McpTool => { ]; }; +/** + * A tool name, typically the first entry in the tuple. Used in logging and deduplication. + */ +componentSchemasTool.toolName = 'componentSchemas'; + export { componentSchemasTool }; diff --git a/src/tool.fetchDocs.ts b/src/tool.fetchDocs.ts index 7d5f450..2e2bea5 100644 --- a/src/tool.fetchDocs.ts +++ b/src/tool.fetchDocs.ts @@ -56,4 +56,9 @@ const fetchDocsTool = (options = getOptions()): McpTool => { ]; }; +/** + * A tool name, typically the first entry in the tuple. Used in logging and deduplication. + */ +fetchDocsTool.toolName = 'fetchDocs'; + export { fetchDocsTool }; diff --git a/src/tool.patternFlyDocs.ts b/src/tool.patternFlyDocs.ts index bec3614..ecb29b6 100644 --- a/src/tool.patternFlyDocs.ts +++ b/src/tool.patternFlyDocs.ts @@ -77,4 +77,9 @@ const usePatternFlyDocsTool = (options = getOptions()): McpTool => { ]; }; +/** + * A tool name, typically the first entry in the tuple. Used in logging and deduplication. + */ +usePatternFlyDocsTool.toolName = 'usePatternFlyDocs'; + export { usePatternFlyDocsTool }; From 0aa79abc625da4038c7515c986a27fa9a65cb6ce Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Sat, 6 Dec 2025 23:12:33 -0500 Subject: [PATCH 2/3] feat: mcp tools as plugins --- README.md | 316 +++++++++++++++++- .../__snapshots__/options.test.ts.snap | 26 ++ src/__tests__/index.test.ts | 1 + src/index.ts | 58 +++- src/options.context.ts | 24 +- src/options.ts | 64 +++- src/server.ts | 14 +- tests/__fixtures__/tool.echo.js | 21 ++ .../__snapshots__/stdioTransport.test.ts.snap | 39 +++ tests/httpTransport.test.ts | 108 +++++- tests/stdioTransport.test.ts | 49 ++- tests/utils/httpTransportClient.ts | 1 + tsconfig.json | 1 + 13 files changed, 699 insertions(+), 23 deletions(-) create mode 100644 tests/__fixtures__/tool.echo.js diff --git a/README.md b/README.md index 855de3a..1b0d872 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ The Model Context Protocol (MCP) is an open standard that enables AI assistants ## Prerequisites - Node.js 20.0.0 or higher + - Note: External tool plugins require Node.js >= 22 at runtime. On Node < 22, the server starts with built‑in tools only and logs a one‑time warning. - NPM (or another Node package manager) ## Installation @@ -83,6 +84,258 @@ Returned content format: - For each entry in urlList, the server loads its content, prefixes it with a header like: `# Documentation from ` and joins multiple entries using a separator: `\n\n---\n\n`. - If an entry fails to load, an inline error message is included for that entry. +### External tools (Plugins) + +Add external tools at startup. External tools run out‑of‑process in a separate Tools Host (Node >= 22). Built‑in tools are always in‑process and register first. + +- Node version gate + - Node < 22 → external tools are skipped with a single startup warning; built‑ins still register. + - Node >= 22 → external tools run out‑of‑process via the Tools Host. + +- CLI + - `--tool ` Add one or more external tools. Repeat the flag or pass a comma‑separated list. + - Examples: `--tool @acme/my-plugin`, `--tool ./plugins/my-tools.js`, `--tool ./a.js,./b.js` + - `--plugin-isolation ` Tools Host permission preset. + - Defaults: `strict` when any `--tool` is provided; otherwise `none`. + +- Behavior + - External tools run in a single Tools Host child process. + - In `strict` isolation (default with externals): network and fs write are denied; fs reads are allow‑listed to your project and resolved plugin directories. + +- Supported `--tool` inputs + - ESM packages (installed in node_modules) + - Local ESM files (paths are normalized to `file://` URLs internally) + +- Not supported as `--tool` inputs + - Raw TypeScript sources (`.ts`) — the Tools Host does not install a TS loader + - Remote `http(s):` or `data:` URLs — these will fail to load and appear in startup warnings/errors + +- Troubleshooting + - If external tools don't appear, verify you're running on Node >= 22 (see Node version gate above) and check startup `load:ack` warnings/errors. + - Startup `load:ack` warnings/errors from plugins are logged when stderr/protocol logging is enabled. + - If `tools/list` fails or `tools/call` rejects due to argument validation (e.g., messages about `safeParseAsync is not a function`), ensure your `inputSchema` is either a valid JSON Schema object or a Zod schema. Plain JSON Schema objects are automatically converted, but malformed schemas may cause issues. See the [Input Schema Format](#input-schema-format) section for details. + +### Embedding the server (Programmatic API) + +You can embed the MCP server inside another Node/TypeScript application and register tools programmatically. + +Tools as plugins can be + - Inline creators, or an array/list of inline creators, provided through the convenience wrapper `createMcpTool`, i.e. `createMcpTool({ name: 'echoAMessage', ... })` or `createMcpTool([{ name: 'echoAMessage', ... }])`. + - Local file paths and local file URLs (Node >= 22 required), i.e. `a string representing a local file path or file URL starting with file://` + - Local NPM package names (Node >= 22 required), i.e. `a string representing a local NPM package name like @loremIpsum/my-plugin` + +> Note: Consuming remote/external files, such as YML, and NPM packages is targeted for the near future. + +Supported export shapes for external modules (Node >= 22 only): + +- Default export: function returning a realized tool tuple. It is called once with ToolOptions and cached. Example shape: `export default function (opts) { return ['name', { description, inputSchema }, handler] }` +- Default export: function returning an array of creator functions. Example shape: `export default function (opts) { return [() => [...], () => [...]] }` +- Default export: array of creator functions. Example shape: `export default [ () => ['name', {...}, handler] ]` +- Fallback: a named export that is an array of creator functions (only used if default export is not present). + +Not supported (Phase A+B): + +- Directly exporting a bare tuple as the module default (wrap it in a function instead) +- Plugin objects like `{ createCreators, createTools }` + +Performance and determinism note: + +- If your default export is a function that returns a tuple, we invoke it once during load with a minimal ToolOptions object and cache the result. Use a creators‑factory (a function returning an array of creators) if you need per‑realization variability by options. + +External module examples (Node >= 22): + +Function returning a tuple (called once with options): + +```js +// plugins/echo.js +export default function createEchoTool(opts) { + return [ + 'echo_plugin_tool', + { description: 'Echo', inputSchema: { additionalProperties: true } }, + async (args) => ({ content: [{ type: 'text', text: JSON.stringify({ args, opts }) }] }) + ]; +} +``` + +Function returning multiple creators: + +```js +// plugins/multi.js +const t1 = () => ['one', { description: 'One', inputSchema: {} }, async () => ({})]; +const t2 = () => ['two', { description: 'Two', inputSchema: {} }, async () => ({})]; + +export default function creators(opts) { + // You can use opts to conditionally include creators + return [t1, t2]; +} +``` + +Array of creators directly: + +```js +// plugins/direct-array.js +export default [ + () => ['hello', { description: 'Hello', inputSchema: {} }, async () => ({})] +]; +``` + +#### Example +```typescript +// app.ts +import { start, createMcpTool, type PfMcpInstance, type PfMcpLogEvent, type ToolCreator } from '@patternfly/patternfly-mcp'; + +// Define a simple inline MCP tool. `createMcpTool` is a convenience wrapper to help you start writing a MCP tool. +const echoTool: ToolCreator = createMcpTool({ + // The unique name of the tool, used in the `tools/list` response, related to the MCP client. + // A MCP client can help Models use this, so make it informative and clear. + name: 'echoAMessage', + + // A short description of the tool, used in the `tools/list` response, related to the MCP client. + // A MCP client can help Models can use this, so make it informative and clear. + description: 'Echo back the provided user message.', + + // The input schema defines the shape of interacting with your handler, related to the Model. + // In this scenario the `args` object has a `string` `message` property intended to be passed back + // towards the tool `handler` when the Model calls it. + inputSchema: { + type: 'object', // Type of the input schema, in this case the object + properties: { message: { type: 'string' } }, // The properties, with types, to pass back to the handler + required: ['message'] // Required properties, in this case `message` + }, + + // The handler, async or sync. The Model calls the handler per the client and inputSchema and inputs the + // `message`. The handler parses the `message` and returns it. The Model receives the parsed `message` + // and uses it. + handler: async (args: { message: string }) => ({ text: `You said: ${args.message}` }) +}); + +async function main() { + // Start the server. + const server: PfMcpInstance = await start({ + // Add one or more in‑process tools directly. Default tools will be registered first. + toolModules: [ + // You can pass: + // - a string module (package or file) for external plugins (Tools Host, Node ≥ 22), or + // - a creator function returned by createMcpTool(...) for in‑process tools. + echoTool + ] + // Optional: enable all logging through stderr and/or protocol. + // logging: { level: 'info', stderr: true }, + }); + + // Optional: observe refined server logs in‑process + server.onLog((event: PfMcpLogEvent) => { + // A good habit to get into is avoiding `console.log` and `console.info` in production paths, they pollute stdio + // communication and can create noise. Use `console.error`, `console.warn`, or `process.stderr.write` instead. + if (event.level !== 'debug') { + // process.stderr.write(`[${event.level}] ${event.msg || ''}\n`); + // console.error(`[${event.level}] ${event.msg || ''}`); + console.warn(`[${event.level}] ${event.msg || ''}`); + } + }); + + // Stop the server after 10 seconds. + setTimeout(async () => server.stop(), 10000); +} + +// Run the program. +main().catch((err) => { + // In programmatic mode, unhandled errors throw unless allowProcessExit=true + console.error(err); + process.exit(1); +}); +``` + +#### Development notes +- Built‑in tools are always registered first. +- Consuming the MCP server comes with a not-so-obvious limitation, avoiding `console.log` and `console.info`. + - In `stdio` server run mode `console.log` and `console.info` can create unnecessary noise between server and client, and potentially the Model. Instead, use `console.error`, `console.warn`, or `process.stderr.write`. + - In `http` server run mode `console.log` and `console.info` can be used, but it's still recommended you get in the habit of avoiding their use. + +### Authoring external tools with `createMcpTool` + +Export an ESM module using `createMcpTool`. The server adapts single or multiple tool definitions automatically. + +Single tool: + +```ts +import { createMcpTool } from '@patternfly/patternfly-mcp'; + +export default createMcpTool({ + name: 'hello', + description: 'Say hello', + inputSchema: { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + }, + async handler({ name }) { + return `Hello, ${name}!`; + } +}); +``` + +Multiple tools: + +```ts +import { createMcpTool } from '@patternfly/patternfly-mcp'; + +export default createMcpTool([ + { name: 'hi', description: 'Greets', inputSchema: { type: 'object' }, handler: () => 'hi' }, + { name: 'bye', description: 'Farewell', inputSchema: { type: 'object' }, handler: () => 'bye' } +]); +``` + +Named group: + +```ts +import { createMcpTool } from '@patternfly/patternfly-mcp'; + +export default createMcpTool({ + name: 'my-plugin', + tools: [ + { name: 'alpha', description: 'A', inputSchema: { type: 'object' }, handler: () => 'A' }, + { name: 'beta', description: 'B', inputSchema: { type: 'object' }, handler: () => 'B' } + ] +}); +``` + +Notes +- External tools must be ESM modules (packages or ESM files). The Tools Host imports your module via `import()`. +- The `handler` receives `args` per your `inputSchema`. A reserved `options?` parameter may be added in a future release; it is not currently passed. + +### Input Schema Format + +The `inputSchema` property accepts either **plain JSON Schema objects** or **Zod schemas**. Both formats are automatically converted to the format required by the MCP SDK. + +**JSON Schema (recommended for simplicity):** +``` +inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' } + }, + required: ['name'] +} +``` + +**Zod Schema (for advanced validation):** +``` +import { z } from 'zod'; + +inputSchema: { + name: z.string(), + age: z.number().optional() +} +``` + +**Important:** The MCP SDK expects Zod-compatible schemas internally. Plain JSON Schema objects are automatically converted to equivalent Zod schemas when tools are registered. This conversion handles common cases like: +- `{ type: 'object', additionalProperties: true }` → `z.object({}).passthrough()` +- Simple object schemas → `z.object({...})` + +If you encounter validation errors, ensure your JSON Schema follows standard JSON Schema format, or use Zod schemas directly for more control. + ## Logging The server uses a `diagnostics_channel`–based logger that keeps STDIO stdout pure by default. No terminal output occurs unless you enable a sink. @@ -333,7 +586,68 @@ npx @modelcontextprotocol/inspector-cli \ ## Environment variables - DOC_MCP_FETCH_TIMEOUT_MS: Milliseconds to wait before aborting an HTTP fetch (default: 15000) -- DOC_MCP_CLEAR_COOLDOWN_MS: Default cooldown value used in internal cache configuration. The current public API does not expose a `clearCache` tool. + +## External tools (plugins) + +You can load external MCP tool modules at runtime using a single CLI flag or via programmatic options. Modules must be ESM-importable (absolute/relative path or package). + +CLI examples (single `--tool` flag): + +```bash +# Single module +npm run start:dev -- --tool ./dist/my-tool.js + +# Multiple modules (repeatable) +npm run start:dev -- --tool ./dist/t1.js --tool ./dist/t2.js + +# Multiple modules (comma-separated) +npm run start:dev -- --tool ./dist/t1.js,./dist/t2.js +``` + +Programmatic usage: + +```ts +import { main } from '@patternfly/patternfly-mcp'; + +await main({ + toolModules: [ + new URL('./dist/t1.js', import.meta.url).toString(), + './dist/t2.js' + ] +}); +``` + +Tools provided via `--tool`/`toolModules` are appended after the built-in tools. + +### Authoring MCP external tools +> Note: External MCP tools require using `Node >= 22` to run the server and ESM modules. TypeScript formatted tools are not directly supported. +> If you do use TypeScript, you can use the `createMcpTool` helper to define your tools as pure ESM modules. + +For `tools-as-plugin` authors, we recommend using the unified helper to define your tools as pure ESM modules: + +```ts +import { createMcpTool } from '@patternfly/patternfly-mcp'; + +export default createMcpTool({ + name: 'hello', + description: 'Say hello', + inputSchema: { type: 'object', properties: { name: { type: 'string' } }, required: ['name'] }, + async handler({ name }) { + return { content: `Hello, ${name}!` }; + } +}); +``` + +Multiple tools in one module: + +```ts +import { createMcpTool } from '@patternfly/patternfly-mcp'; + +export default createMcpTool([ + { name: 'hello', description: 'Hi', inputSchema: {}, handler: () => 'hi' }, + { name: 'bye', description: 'Bye', inputSchema: {}, handler: () => 'bye' } +]); +``` ## Programmatic usage (advanced) diff --git a/src/__tests__/__snapshots__/options.test.ts.snap b/src/__tests__/__snapshots__/options.test.ts.snap index 3f817c6..6e56c0e 100644 --- a/src/__tests__/__snapshots__/options.test.ts.snap +++ b/src/__tests__/__snapshots__/options.test.ts.snap @@ -17,6 +17,8 @@ exports[`parseCliOptions should attempt to parse args with --allowed-hosts 1`] = "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -37,6 +39,8 @@ exports[`parseCliOptions should attempt to parse args with --allowed-origins 1`] "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -52,6 +56,8 @@ exports[`parseCliOptions should attempt to parse args with --docs-host flag 1`] "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -69,6 +75,8 @@ exports[`parseCliOptions should attempt to parse args with --http and --host 1`] "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -86,6 +94,8 @@ exports[`parseCliOptions should attempt to parse args with --http and --port 1`] "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -101,6 +111,8 @@ exports[`parseCliOptions should attempt to parse args with --http and invalid -- "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -116,6 +128,8 @@ exports[`parseCliOptions should attempt to parse args with --http flag 1`] = ` "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -131,6 +145,8 @@ exports[`parseCliOptions should attempt to parse args with --log-level flag 1`] "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -146,6 +162,8 @@ exports[`parseCliOptions should attempt to parse args with --log-stderr flag and "stderr": true, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -161,6 +179,8 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] = "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -176,6 +196,8 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag and -- "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -191,6 +213,8 @@ exports[`parseCliOptions should attempt to parse args with other arguments 1`] = "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; @@ -206,5 +230,7 @@ exports[`parseCliOptions should attempt to parse args without --docs-host flag 1 "stderr": false, "transport": "stdio", }, + "pluginIsolation": undefined, + "toolModules": [], } `; diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 8c18b89..a630608 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -8,6 +8,7 @@ import { runServer } from '../server'; jest.mock('../options'); jest.mock('../options.context'); jest.mock('../server'); +jest.mock('../server.tools'); const mockParseCliOptions = parseCliOptions as jest.MockedFunction; const mockSetOptions = setOptions as jest.MockedFunction; diff --git a/src/index.ts b/src/index.ts index c749a8b..846833a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,12 +8,13 @@ import { type ServerOnLogHandler, type ServerLogEvent } from './server'; +import { createMcpTool, type ToolCreator, type ToolModule, type ToolConfig, type MultiToolConfig } from './server.toolsUser'; +// import { type ToolOptions } from './options.tools'; +// import { createMcpTool, type ToolCreator, type ToolConfig, type MultiToolConfig } from './server.toolsUser'; /** * Options for "programmatic" use. Extends the `DefaultOptions` interface. * - * @interface - * * @property {('cli' | 'programmatic' | 'test')} [mode] - Optional string property that specifies the mode of operation. * Defaults to `'programmatic'`. * - `'cli'`: Functionality is being executed in a cli context. Allows process exits. @@ -36,12 +37,40 @@ type PfMcpOptions = DefaultOptionsOverrides & { type PfMcpSettings = Pick; /** - * Main function - CLI entry point with optional programmatic overrides + * Server instance with shutdown capability + * + * @alias ServerInstance + */ +type PfMcpInstance = ServerInstance; + +/** + * Subscribes a handler function, `PfMcpOnLogHandler`, to server logs. Automatically unsubscribed on server shutdown. + * + * @alias ServerOnLog + */ +type PfMcpOnLog = ServerOnLog; + +/** + * The handler function passed by `onLog`, `PfMcpOnLog`, to subscribe to server logs. Automatically unsubscribed on server shutdown. + * + * @alias ServerOnLogHandler + */ +type PfMcpOnLogHandler = ServerOnLogHandler; + +/** + * The log event passed to the `onLog` handler, `PfMcpOnLogHandler`. + * + * @alias ServerLogEvent + */ +type PfMcpLogEvent = ServerLogEvent; + +/** + * Main function - Programmatic and CLI entry point with optional overrides * * @param [pfMcpOptions] - User configurable options * @param [pfMcpSettings] - MCP server settings * - * @returns {Promise} Server-instance with shutdown capability + * @returns {Promise} Server-instance with shutdown capability * * @throws {Error} If the server fails to start or any error occurs during initialization, * and `allowProcessExit` is set to `false`, the error will be thrown rather than exiting @@ -50,7 +79,7 @@ type PfMcpSettings = Pick; const main = async ( pfMcpOptions: PfMcpOptions = {}, pfMcpSettings: PfMcpSettings = {} -): Promise => { +): Promise => { const { mode, ...options } = pfMcpOptions; const { allowProcessExit } = pfMcpSettings; @@ -65,8 +94,10 @@ const main = async ( // use runWithSession to enable session in listeners return await runWithSession(session, async () => - // `runServer` doesn't require it, but `memo` does for "uniqueness", pass in the merged options for a hashable argument - runServer.memo(mergedOptions, { allowProcessExit: updatedAllowProcessExit })); + // `runServer` doesn't require options in the memo key, but we pass fully-merged options for stable hashing + await runServer.memo(mergedOptions, { + allowProcessExit: updatedAllowProcessExit + })); } catch (error) { console.error('Failed to start server:', error); @@ -79,13 +110,18 @@ const main = async ( }; export { + createMcpTool, main, main as start, type CliOptions, type PfMcpOptions, type PfMcpSettings, - type ServerInstance, - type ServerLogEvent, - type ServerOnLog, - type ServerOnLogHandler + type PfMcpInstance, + type PfMcpLogEvent, + type PfMcpOnLog, + type PfMcpOnLogHandler, + type ToolCreator, + type ToolModule, + type ToolConfig, + type MultiToolConfig }; diff --git a/src/options.context.ts b/src/options.context.ts index 6739f0e..f4a794e 100644 --- a/src/options.context.ts +++ b/src/options.context.ts @@ -67,6 +67,12 @@ const optionsContext = new AsyncLocalStorage(); const setOptions = (options?: DefaultOptionsOverrides): GlobalOptions => { const base = mergeObjects(DEFAULT_OPTIONS, options, { allowNullValues: false, allowUndefinedValues: false }); const baseLogging = isPlainObject(base.logging) ? base.logging : DEFAULT_OPTIONS.logging; + + // We handle plugin isolation here to account for both CLI and programmatic usage. + const requestedPluginIsolation = options?.pluginIsolation; + const defaultPluginIsolation = Array.isArray(base.toolModules) && base.toolModules.length > 0 ? 'strict' : 'none'; + const pluginIsolation = requestedPluginIsolation ?? defaultPluginIsolation; + const merged: GlobalOptions = { ...base, logging: { @@ -76,11 +82,21 @@ const setOptions = (options?: DefaultOptionsOverrides): GlobalOptions => { protocol: baseLogging.protocol, transport: baseLogging.transport }, + pluginIsolation, resourceMemoOptions: DEFAULT_OPTIONS.resourceMemoOptions, toolMemoOptions: DEFAULT_OPTIONS.toolMemoOptions }; - const frozen = freezeObject(structuredClone(merged)); + // AFTER + const originalToolModules = Array.isArray(merged.toolModules) ? merged.toolModules : []; + + // Avoid cloning functions in toolModules + const mergedCloneSafe = { ...merged, toolModules: [] as unknown[] }; + const cloned = structuredClone(mergedCloneSafe); + + // Restore the non‑cloneable array reference + const restored: GlobalOptions = { ...cloned, toolModules: originalToolModules } as GlobalOptions; + const frozen = freezeObject(restored); optionsContext.enterWith(frozen); @@ -123,7 +139,11 @@ const runWithOptions = async ( options: GlobalOptions, callback: () => TReturn | Promise ) => { - const frozen = freezeObject(structuredClone(options)); + const originalToolModules = Array.isArray((options as any).toolModules) ? (options as any).toolModules : []; + const optionsCloneSafe = { ...(options as any), toolModules: [] as unknown[] }; + const cloned = structuredClone(optionsCloneSafe); + const restored = { ...cloned, toolModules: originalToolModules } as GlobalOptions; + const frozen = freezeObject(restored); return optionsContext.run(frozen, callback); }; diff --git a/src/options.ts b/src/options.ts index ebced89..db17b7d 100644 --- a/src/options.ts +++ b/src/options.ts @@ -22,6 +22,13 @@ type CliOptions = { http?: Partial; isHttp: boolean; logging: Partial; + toolModules: string[]; + + /** + * Isolation preset for external plugins (CLI-provided). If omitted, defaults + * to 'strict' when external tools are requested, otherwise 'none'. + */ + pluginIsolation: 'none' | 'strict' | undefined; }; /** @@ -72,6 +79,7 @@ const getArgValue = (flag: string, { defaultValue, argv = process.argv }: { defa * - `--host`: The host name specified via `--host` * - `--allowed-origins`: List of allowed origins derived from the `--allowed-origins` parameter, split by commas, or undefined if not provided. * - `--allowed-hosts`: List of allowed hosts derived from the `--allowed-hosts` parameter, split by commas, or undefined if not provided. + * - `--plugin-isolation `: Isolation preset for external plugins. * * @param [argv] - Command-line arguments to parse. Defaults to `process.argv`. * @returns Parsed command-line options. @@ -133,7 +141,61 @@ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { } } - return { docsHost, logging, isHttp, http }; + // Parse external tool modules: single canonical flag `--tool` + // Supported forms: + // --tool a --tool b (repeatable) + // --tool a,b (comma-separated) + const toolModules: string[] = []; + const seenSpecs = new Set(); + + const addSpec = (spec?: string) => { + const trimmed = String(spec || '').trim(); + + if (!trimmed || seenSpecs.has(trimmed)) { + return; + } + + seenSpecs.add(trimmed); + toolModules.push(trimmed); + }; + + for (let argIndex = 0; argIndex < argv.length; argIndex += 1) { + const token = argv[argIndex]; + const next = argv[argIndex + 1]; + + if (token === '--tool' && typeof next === 'string' && !next.startsWith('-')) { + next + .split(',') + .map(value => value.trim()) + .filter(Boolean) + .forEach(addSpec); + + argIndex += 1; + } + } + + // Parse isolation preset: --plugin-isolation + let pluginIsolation: CliOptions['pluginIsolation'];// = DEFAULT_OPTIONS.pluginIsolation; + const isolationIndex = argv.indexOf('--plugin-isolation'); + + if (isolationIndex >= 0) { + const val = String(argv[isolationIndex + 1] || '').toLowerCase(); + + switch (val) { + case 'none': + case 'strict': + pluginIsolation = val; + } + } + + return { + docsHost, + logging, + isHttp, + http, + toolModules, + pluginIsolation + }; }; export { diff --git a/src/server.ts b/src/server.ts index 4a4179c..9e7ed9f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -7,6 +7,7 @@ import { startHttpTransport, type HttpServerHandle } from './server.http'; import { memo } from './server.caching'; import { log, type LogEvent } from './logger'; import { createServerLogger } from './server.logger'; +import { composeTools, sendToolsHostShutdown } from './server.tools'; import { type GlobalOptions } from './options'; import { getOptions, @@ -129,6 +130,10 @@ const runServer = async (options: ServerOptions = getOptions(), { await server?.close(); running = false; + try { + await sendToolsHostShutdown(); + } catch {} + log.info(`${options.name} closed!\n`); unsubscribeServerLogger?.(); @@ -167,6 +172,9 @@ const runServer = async (options: ServerOptions = getOptions(), { ); } + // Combine built-in tools with custom ones after logging is set up. + const updatedTools = await composeTools(tools); + if (subUnsub) { const { subscribe, unsubscribe } = subUnsub; @@ -177,7 +185,7 @@ const runServer = async (options: ServerOptions = getOptions(), { onLogSetup = (handler: ServerOnLogHandler) => subscribe(handler); } - tools.forEach(toolCreator => { + updatedTools.forEach(toolCreator => { const [name, schema, callback] = toolCreator(options); // Do NOT normalize schemas here. This is by design and is a fallback check for malformed schemas. const isZod = isZodSchema(schema?.inputSchema) || isZodRawShape(schema?.inputSchema); @@ -291,6 +299,10 @@ runServer.memo = memo( // Avoid engaging the contextual log channel on rollout. console.error(`Error stopping server: ${error}`); } + + try { + await sendToolsHostShutdown(); + } catch {} } } else { // Avoid engaging the contextual log channel on rollout. diff --git a/tests/__fixtures__/tool.echo.js b/tests/__fixtures__/tool.echo.js new file mode 100644 index 0000000..1a018c0 --- /dev/null +++ b/tests/__fixtures__/tool.echo.js @@ -0,0 +1,21 @@ +// Fixture exports a creator function directly; + +const echo_plugin_tool = options => [ + 'echo_plugin_tool', + { + description: 'Echo back the provided args, but with a different description', + inputSchema: { additionalProperties: true } + }, + args => ({ + args, + options, + content: [ + { + type: 'text', + text: JSON.stringify(args) + } + ] + }) +]; + +export { echo_plugin_tool as default, echo_plugin_tool }; diff --git a/tests/__snapshots__/stdioTransport.test.ts.snap b/tests/__snapshots__/stdioTransport.test.ts.snap index 0f41955..5ca2135 100644 --- a/tests/__snapshots__/stdioTransport.test.ts.snap +++ b/tests/__snapshots__/stdioTransport.test.ts.snap @@ -439,3 +439,42 @@ exports[`PatternFly MCP, STDIO should expose expected tools and stable shape 1`] ], } `; + +exports[`Tools should access a new tool 1`] = ` +[ + "[INFO]: Server logging enabled. +", + "[INFO]: Registered tool: usePatternFlyDocs +", + "[INFO]: Registered tool: fetchDocs +", + "[INFO]: Registered tool: componentSchemas +", + "[INFO]: Registered tool: echo_plugin_tool +", + "[INFO]: @patternfly/patternfly-mcp server running on stdio transport +", +] +`; + +exports[`Tools should interact with the new tool 1`] = ` +{ + "args": { + "dolor": "sit amet", + "lorem": "ipsum", + "type": "echo", + }, + "content": [ + { + "text": "{"type":"echo","lorem":"ipsum","dolor":"sit amet"}", + "type": "text", + }, + ], + "options": { + "nodeMajor": 22, + "repoName": "patternfly-mcp", + "serverName": "@patternfly/patternfly-mcp", + "serverVersion": "0.4.0", + }, +} +`; diff --git a/tests/httpTransport.test.ts b/tests/httpTransport.test.ts index 38fbd29..c88ee1b 100644 --- a/tests/httpTransport.test.ts +++ b/tests/httpTransport.test.ts @@ -1,12 +1,13 @@ /** * Requires: npm run build prior to running Jest. */ -import { - startServer, - type HttpTransportClient, - type RpcRequest -} from './utils/httpTransportClient'; +// import { resolve } from 'node:path'; +// import { pathToFileURL } from 'node:url'; +// @ts-ignore - dist/index.js isn't necessarily built yet, remember to build before running tests +import { createMcpTool } from '../dist/index.js'; +import { startServer, type HttpTransportClient, type RpcRequest } from './utils/httpTransportClient'; import { setupFetchMock } from './utils/fetchMock'; +// Use public types from dist to avoid type identity mismatches between src and dist describe('PatternFly MCP, HTTP Transport', () => { let FETCH_MOCK: Awaited> | undefined; @@ -41,7 +42,7 @@ describe('PatternFly MCP, HTTP Transport', () => { excludePorts: [5001] }); - CLIENT = await startServer({ http: { port: 5001 } }); + CLIENT = await startServer({ http: { port: 5001 }, logging: { level: 'debug', protocol: true } }); }); afterAll(async () => { @@ -96,6 +97,9 @@ describe('PatternFly MCP, HTTP Transport', () => { const response = await CLIENT?.send(req); const text = response?.result?.content?.[0]?.text || ''; + // expect(CLIENT?.logs()).toMatchSnapshot(); + // expect(CLIENT?.protocolLogs()).toMatchSnapshot(); + expect(text.startsWith('# Documentation from')).toBe(true); expect(text).toMatchSnapshot(); }); @@ -125,3 +129,95 @@ describe('PatternFly MCP, HTTP Transport', () => { CLIENT.close(); }); }); + +describe('Inline tools over HTTP', () => { + let CLIENT: HttpTransportClient | undefined; + + afterAll(async () => { + if (CLIENT) { + await CLIENT.close(); + } + }); + + it.each([ + { + description: 'inline tool module', + port: 5011, + toolName: 'inline_module', + tool: createMcpTool({ + name: 'inline_module', + description: 'Create inline', + inputSchema: { additionalProperties: true }, + handler: (args: any) => ({ content: [{ type: 'text', text: JSON.stringify(args) }] }) + }) + }, + { + description: 'inline tool creator', + port: 5012, + toolName: 'inline_creator', + tool: (_options: any) => [ + 'inline_creator', + { + description: 'Func inline', + inputSchema: { additionalProperties: true } + }, + (args: any) => ({ content: [{ type: 'text', text: JSON.stringify(args) }] }) + ] + }, + { + description: 'inline object', + port: 5013, + toolName: 'inline_obj', + tool: { + name: 'inline_obj', + description: 'Obj inline', + inputSchema: { additionalProperties: true }, + handler: (args: any) => ({ content: [{ type: 'text', text: JSON.stringify(args) }] }) + } + }, + { + description: 'inline tuple', + port: 5014, + toolName: 'inline_tuple', + tool: [ + 'inline_tuple', + { + description: 'Tuple inline', + inputSchema: { additionalProperties: true } + }, + (args: any) => ({ content: [{ type: 'text', text: JSON.stringify(args) }] }) + ] + } + ])('should register and invoke an inline tool module, $description', async ({ port, tool, toolName }) => { + CLIENT = await startServer( + { + http: { port }, + isHttp: true, + logging: { level: 'info', protocol: true }, + toolModules: [tool as any] + }, + { allowProcessExit: false } + ); + + const list = await CLIENT.send({ method: 'tools/list', params: {} }); + const names = (list?.result?.tools || []).map((tool: any) => tool.name); + + expect(names).toEqual(expect.arrayContaining([toolName])); + + const req = { + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { + name: toolName, + arguments: { x: 1, y: 'z' } + } + } as RpcRequest; + + const res = await CLIENT.send(req); + + expect(res?.result?.content?.[0]?.text).toContain('"x":1'); + + await CLIENT.close(); + }); +}); diff --git a/tests/stdioTransport.test.ts b/tests/stdioTransport.test.ts index e06c977..791b17b 100644 --- a/tests/stdioTransport.test.ts +++ b/tests/stdioTransport.test.ts @@ -1,6 +1,8 @@ /** * Requires: npm run build prior to running Jest. */ +import { resolve } from 'node:path'; +import { pathToFileURL } from 'node:url'; import { startServer, type StdioTransportClient, @@ -87,7 +89,7 @@ describe('PatternFly MCP, STDIO', () => { } } as RpcRequest; - const response = await CLIENT?.send(req); + const response = await CLIENT.send(req); const text = response?.result?.content?.[0]?.text || ''; expect(text.startsWith('# Documentation from')).toBe(true); @@ -172,3 +174,48 @@ describe('Logging', () => { await CLIENT.stop(); }); }); + +describe('Tools', () => { + let CLIENT: StdioTransportClient; + + beforeEach(async () => { + const abs = resolve(process.cwd(), 'tests/__fixtures__/tool.echo.js'); + const url = pathToFileURL(abs).href; + + CLIENT = await startServer({ args: ['--log-stderr', '--plugin-isolation', 'strict', '--tool', url] }); + }); + + afterEach(async () => CLIENT.stop()); + + it('should access a new tool', async () => { + const req = { + method: 'tools/list', + params: {} + }; + + const resp = await CLIENT.send(req); + const names = (resp?.result?.tools ?? []).map((tool: any) => tool.name); + + expect(names).toContain('echo_plugin_tool'); + expect(CLIENT.logs()).toMatchSnapshot(); + }); + + it('should interact with the new tool', async () => { + const req = { + method: 'tools/call', + params: { + name: 'echo_plugin_tool', + arguments: { + type: 'echo', + lorem: 'ipsum', + dolor: 'sit amet' + } + } + }; + + const resp: any = await CLIENT.send(req); + + expect(resp.result).toMatchSnapshot(); + // expect(resp.result.content[0].text).toMatchSnapshot(); + }); +}); diff --git a/tests/utils/httpTransportClient.ts b/tests/utils/httpTransportClient.ts index 54ae40a..ca688c7 100644 --- a/tests/utils/httpTransportClient.ts +++ b/tests/utils/httpTransportClient.ts @@ -16,6 +16,7 @@ export type StartHttpServerOptions = { http?: Partial; isHttp?: boolean; logging?: Partial & { level?: LoggingLevel }; + toolModules?: PfMcpOptions['toolModules']; }; export type StartHttpServerSettings = PfMcpSettings; diff --git a/tsconfig.json b/tsconfig.json index b44ca37..8c47af0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -20,6 +20,7 @@ "exactOptionalPropertyTypes": true, "resolveJsonModule": true, "noEmit": true, + "stripInternal": true, "rootDirs": ["./src", "./tests"] }, "include": [ From c85a9499d06428d804adfca7d9cb5ed2d9ee5362 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Tue, 23 Dec 2025 15:40:30 -0500 Subject: [PATCH 3/3] test: review updates --- .../__snapshots__/server.test.ts.snap | 30 +++++++++++++++++++ tests/__fixtures__/tool.echo.js | 2 +- .../__snapshots__/stdioTransport.test.ts.snap | 2 ++ tests/stdioTransport.test.ts | 2 +- 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/__tests__/__snapshots__/server.test.ts.snap b/src/__tests__/__snapshots__/server.test.ts.snap index f81dc88..57789b8 100644 --- a/src/__tests__/__snapshots__/server.test.ts.snap +++ b/src/__tests__/__snapshots__/server.test.ts.snap @@ -6,6 +6,9 @@ exports[`runServer should allow server to be stopped, http stop server: diagnost [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "Registered tool: usePatternFlyDocs", ], @@ -35,6 +38,9 @@ exports[`runServer should allow server to be stopped, stdio stop server: diagnos [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "Registered tool: usePatternFlyDocs", ], @@ -64,6 +70,9 @@ exports[`runServer should attempt to run server, create transport, connect, and [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "test-server-4 server running on stdio transport", ], @@ -98,6 +107,9 @@ exports[`runServer should attempt to run server, disable SIGINT handler: diagnos [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "test-server-7 server running on stdio transport", ], @@ -127,6 +139,9 @@ exports[`runServer should attempt to run server, enable SIGINT handler explicitl [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "test-server-8 server running on stdio transport", ], @@ -161,6 +176,9 @@ exports[`runServer should attempt to run server, register a tool: diagnostics 1` [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "Registered tool: loremIpsum", ], @@ -203,6 +221,9 @@ exports[`runServer should attempt to run server, register multiple tools: diagno [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "Registered tool: loremIpsum", ], @@ -252,6 +273,9 @@ exports[`runServer should attempt to run server, use custom options: diagnostics [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "test-server-3 server running on stdio transport", ], @@ -286,6 +310,9 @@ exports[`runServer should attempt to run server, use default tools, http: diagno [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "Registered tool: usePatternFlyDocs", ], @@ -333,6 +360,9 @@ exports[`runServer should attempt to run server, use default tools, stdio: diagn [ "Server logging enabled.", ], + [ + "No external tools loaded.", + ], [ "Registered tool: usePatternFlyDocs", ], diff --git a/tests/__fixtures__/tool.echo.js b/tests/__fixtures__/tool.echo.js index 1a018c0..2453c24 100644 --- a/tests/__fixtures__/tool.echo.js +++ b/tests/__fixtures__/tool.echo.js @@ -18,4 +18,4 @@ const echo_plugin_tool = options => [ }) ]; -export { echo_plugin_tool as default, echo_plugin_tool }; +export default echo_plugin_tool; diff --git a/tests/__snapshots__/stdioTransport.test.ts.snap b/tests/__snapshots__/stdioTransport.test.ts.snap index 5ca2135..67cc093 100644 --- a/tests/__snapshots__/stdioTransport.test.ts.snap +++ b/tests/__snapshots__/stdioTransport.test.ts.snap @@ -271,6 +271,8 @@ exports[`Logging should allow setting logging options, default 1`] = `[]`; exports[`Logging should allow setting logging options, stderr 1`] = ` [ "[INFO]: Server logging enabled. +", + "[INFO]: No external tools loaded. ", "[INFO]: Registered tool: usePatternFlyDocs ", diff --git a/tests/stdioTransport.test.ts b/tests/stdioTransport.test.ts index 791b17b..bfbb1e8 100644 --- a/tests/stdioTransport.test.ts +++ b/tests/stdioTransport.test.ts @@ -196,8 +196,8 @@ describe('Tools', () => { const resp = await CLIENT.send(req); const names = (resp?.result?.tools ?? []).map((tool: any) => tool.name); - expect(names).toContain('echo_plugin_tool'); expect(CLIENT.logs()).toMatchSnapshot(); + expect(names).toContain('echo_plugin_tool'); }); it('should interact with the new tool', async () => {