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 556dab7..a19ef96 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.4.0", "license": "MIT", "dependencies": { - "@modelcontextprotocol/sdk": "1.24.2", + "@modelcontextprotocol/sdk": "1.24.3", "@patternfly/patternfly-component-schemas": "1.2.0", "fastest-levenshtein": "1.0.16", "pid-port": "2.0.0", @@ -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", @@ -1978,9 +1979,9 @@ } }, "node_modules/@modelcontextprotocol/sdk": { - "version": "1.24.2", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.24.2.tgz", - "integrity": "sha512-hS/kzSfchqzvUeJUsdiDHi84/kNhLIZaZ6coGQVwbYIelOBbcAwUohUfaQTLa1MvFOK/jbTnGFzraHSFwB7pjQ==", + "version": "1.24.3", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.24.3.tgz", + "integrity": "sha512-YgSHW29fuzKKAHTGe9zjNoo+yF8KaQPzDC2W9Pv41E7/57IfY+AMGJ/aDFlgTLcVVELoggKE4syABCE75u3NCw==", "license": "MIT", "dependencies": { "ajv": "^8.17.1", @@ -10753,6 +10754,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 234ae61..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", @@ -47,7 +53,7 @@ "author": "Red Hat", "license": "MIT", "dependencies": { - "@modelcontextprotocol/sdk": "1.24.2", + "@modelcontextprotocol/sdk": "1.24.3", "@patternfly/patternfly-component-schemas": "1.2.0", "fastest-levenshtein": "1.0.16", "pid-port": "2.0.0", @@ -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..cc3b28c --- /dev/null +++ b/src/__tests__/__snapshots__/server.tools.test.ts.snap @@ -0,0 +1,392 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +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[`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.tools.test.ts b/src/__tests__/server.tools.test.ts new file mode 100644 index 0000000..c7d6923 --- /dev/null +++ b/src/__tests__/server.tools.test.ts @@ -0,0 +1,1095 @@ +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); + }); +}); + +describe('computeFsReadAllowlist', () => { + it('should return a list of allowed paths', () => { + const toolModules = ['@scope/pkg', resolve(process.cwd(), 'package.json')]; + + expect(computeFsReadAllowlist({ toolModules, contextUrl: 'file://', contextPath: '/' } as any)).toEqual(['/']); + }); +}); + +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', () => { + it('should exist', () => { + // placeholder test + expect(sendToolsHostShutdown).toBeDefined(); + }); +}); + +describe('composeTools', () => { + it('should exist', () => { + // placeholder test + expect(composeTools).toBeDefined(); + }); +}); + +/* +import { spawn, type ChildProcess } from 'node:child_process'; +import { + composeTools, + logWarningsErrors, + sendToolsHostShutdown +} from '../server.tools'; +import { builtinTools } from '../server'; +import { log } from '../logger'; +import { getOptions, getSessionOptions } from '../options.context'; +import { send, awaitIpc, type IpcResponse } from '../server.toolsIpc'; +import { DEFAULT_OPTIONS } from '../options.defaults'; +// import { type ToolDescriptor } from '../server.toolsIpc'; + +// Mock dependencies +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('../options.context', () => ({ + getOptions: jest.fn(), + getSessionOptions: jest.fn(), + setOptions: jest.fn(), + runWithSession: jest.fn(), + runWithOptions: jest.fn() +})); + +jest.mock('../server.toolsIpc', () => { + const actual = jest.requireActual('../server.toolsIpc'); + + return { + ...actual, + makeId: jest.fn(() => 'mock-id'), + send: jest.fn().mockReturnValue(true), + awaitIpc: jest.fn() + }; +}); + +jest.mock('node:child_process', () => ({ + spawn: jest.fn() +})); + +// Mock import.meta.resolve for #toolsHost to avoid test failures +// We'll handle this by ensuring the mock returns a valid path + +jest.mock('node:url', () => { + const actual = jest.requireActual('node:url'); + + return { + ...actual, + fileURLToPath: jest.fn((url: string) => { + if (typeof url === 'string' && url.includes('toolsHost')) { + return '/mock/path/to/toolsHost.js'; + } + + return actual.fileURLToPath(url); + }), + pathToFileURL: actual.pathToFileURL + }; +}); + +jest.mock('node:fs', () => { + const actual = jest.requireActual('node:fs'); + + return { + ...actual, + realpathSync: jest.fn((path: string) => path) + }; +}); + +const MockLog = log as jest.MockedObject; +const MockGetOptions = getOptions as jest.MockedFunction; +const MockGetSessionOptions = getSessionOptions as jest.MockedFunction; +const MockSpawn = spawn as jest.MockedFunction; +const MockSend = send as jest.MockedFunction; +const MockAwaitIpc = awaitIpc as jest.MockedFunction; + +describe('logWarningsErrors', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'with warnings only', + warnings: ['Warning 1', 'Warning 2'], + errors: [], + expectedWarnCalls: 1 + }, + { + description: 'with errors only', + warnings: [], + errors: ['Error 1', 'Error 2'], + expectedWarnCalls: 1 + }, + { + description: 'with both warnings and errors', + warnings: ['Warning 1'], + errors: ['Error 1'], + expectedWarnCalls: 2 + }, + { + description: 'with empty arrays', + warnings: [], + errors: [], + expectedWarnCalls: 0 + }, + { + description: 'with undefined warnings and errors', + warnings: undefined, + errors: undefined, + expectedWarnCalls: 0 + }, + { + description: 'with single warning', + warnings: ['Single warning'], + errors: [], + expectedWarnCalls: 1 + }, + { + description: 'with single error', + warnings: [], + errors: ['Single error'], + expectedWarnCalls: 1 + } + ])('should log warnings and errors, $description', ({ warnings, errors, expectedWarnCalls }) => { + const options: { warnings?: string[]; errors?: string[] } = {}; + + if (warnings !== undefined) { + options.warnings = warnings; + } + if (errors !== undefined) { + options.errors = errors; + } + logWarningsErrors(options); + + expect(MockLog.warn).toHaveBeenCalledTimes(expectedWarnCalls); + if (warnings && warnings.length > 0) { + expect(MockLog.warn).toHaveBeenCalledWith( + expect.stringContaining(`Tools load warnings (${warnings.length})`) + ); + } + if (errors && errors.length > 0) { + expect(MockLog.warn).toHaveBeenCalledWith( + expect.stringContaining(`Tools load errors (${errors.length})`) + ); + } + }); + + it('should format warning messages correctly', () => { + logWarningsErrors({ warnings: ['Warning 1', 'Warning 2'] }); + + expect(MockLog.warn).toHaveBeenCalledWith( + expect.stringContaining('Warning 1') + ); + expect(MockLog.warn).toHaveBeenCalledWith( + expect.stringContaining('Warning 2') + ); + }); + + it('should format error messages correctly', () => { + logWarningsErrors({ errors: ['Error 1', 'Error 2'] }); + + expect(MockLog.warn).toHaveBeenCalledWith( + expect.stringContaining('Error 1') + ); + expect(MockLog.warn).toHaveBeenCalledWith( + expect.stringContaining('Error 2') + ); + }); +}); + + */ +/* +describe('normalizeToolModules', () => { + beforeEach(() => { + jest.clearAllMocks(); + MockGetOptions.mockReturnValue({ + contextPath: '/test/path', + toolModules: [] + } as any); + }); + + it.each([ + { + description: 'file: URL', + toolModules: ['file:///test/module.js'], + expected: ['file:///test/module.js'] + }, + { + description: 'http: URL', + toolModules: ['http://example.com/module.js'], + expected: ['http://example.com/module.js'] + }, + { + description: 'https: URL', + toolModules: ['https://example.com/module.js'], + expected: ['https://example.com/module.js'] + }, + { + description: 'data: URL', + toolModules: ['data:text/javascript,export default {}'], + expected: ['data:text/javascript,export default {}'] + }, + { + description: 'node: protocol', + toolModules: ['node:fs'], + expected: ['node:fs'] + }, + { + description: 'relative path starting with ./', + toolModules: ['./module.js'], + contextPath: '/test/path', + expectedPattern: 'file://' + }, + { + description: 'relative path starting with ../', + toolModules: ['../module.js'], + contextPath: '/test/path', + expectedPattern: 'file://' + }, + { + description: 'absolute path on Unix', + toolModules: ['/absolute/path/module.js'], + contextPath: '/test/path', + expectedPattern: 'file://' + }, + { + description: 'absolute path on Windows', + toolModules: ['C:\\absolute\\path\\module.js'], + contextPath: '/test/path', + expectedPattern: 'file://' + }, + { + description: 'package name', + toolModules: ['@scope/package'], + expected: ['@scope/package'] + }, + { + description: 'scoped package name', + toolModules: ['@patternfly/tools'], + expected: ['@patternfly/tools'] + }, + { + description: 'empty array', + toolModules: [], + expected: [] + } + ])('should normalize tool modules, $description', ({ toolModules, contextPath, expected, expectedPattern }) => { + MockGetOptions.mockReturnValue({ + contextPath: contextPath || '/test/path', + toolModules + } as any); + + const result = normalizeToolModules(); + + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBe(toolModules.length); + + if (expected) { + expect(result).toEqual(expected); + } else if (expectedPattern) { + result.forEach(url => { + expect(url).toMatch(new RegExp(expectedPattern)); + }); + } + }); + + it('should handle multiple mixed module types', () => { + MockGetOptions.mockReturnValue({ + contextPath: '/test/path', + toolModules: [ + 'file:///absolute/module.js', + './relative/module.js', + '@scope/package', + 'https://example.com/module.js' + ] + } as any); + + const result = normalizeToolModules(); + + expect(result.length).toBe(4); + expect(result[0]).toBe('file:///absolute/module.js'); + expect(result[1]).toMatch(/^file:\/\//); + expect(result[2]).toBe('@scope/package'); + expect(result[3]).toBe('https://example.com/module.js'); + }); +}); +*/ +/* +describe('sendToolsHostShutdown', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + + MockGetOptions.mockReturnValue({ + pluginHost: DEFAULT_OPTIONS.pluginHost + } as any); + + MockGetSessionOptions.mockReturnValue({ + sessionId: 'test-session-id', + channelName: 'test-channel' + } as any); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it.each([ + { + description: 'with default grace period', + pluginHost: {}, + expectedGracePeriod: 0 + }, + { + description: 'with custom grace period', + pluginHost: { gracePeriodMs: 1000 }, + expectedGracePeriod: 1000 + }, + { + description: 'with zero grace period', + pluginHost: { gracePeriodMs: 0 }, + expectedGracePeriod: 0 + } + ])('should shutdown tools host, $description', async ({ pluginHost }) => { + MockGetOptions.mockReturnValue({ + pluginHost: { ...DEFAULT_OPTIONS.pluginHost, ...pluginHost } + } as any); + + // Since we can't directly access activeHostsBySession, we'll test + // that the function handles the case when no host exists + await sendToolsHostShutdown(); + + // Should not throw when no host exists + expect(MockSend).not.toHaveBeenCalled(); + }); + + it('should not throw when no active host exists', async () => { + await expect(sendToolsHostShutdown()).resolves.not.toThrow(); + }); +}); + +describe('composeTools', () => { + let mockChild: ChildProcess & { + kill: jest.Mock; + killed: boolean; + on: jest.Mock; + once: jest.Mock; + off: jest.Mock; + send: jest.Mock; + }; + + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + + // Mock import.meta.resolve for #toolsHost + const originalResolve = import.meta.resolve; + + try { + Object.defineProperty(import.meta, 'resolve', { + value: (spec: string) => { + if (spec === '#toolsHost') { + return 'file:///mock/path/to/toolsHost.js'; + } + + return originalResolve.call(import.meta, spec); + }, + writable: true, + configurable: true + }); + } catch { + // If we can't mock import.meta.resolve, tests that require it will fail gracefully + } + + mockChild = { + kill: jest.fn(), + killed: false, + on: jest.fn(), + once: jest.fn(), + off: jest.fn(), + send: jest.fn().mockReturnValue(true), + pid: 123, + connected: true, + disconnect: jest.fn(), + exitCode: null, + signalCode: null, + channel: null, + stdin: null, + stdout: null, + stderr: null, + stdio: [], + spawnfile: '', + spawnargs: [] + } as any; + + MockSpawn.mockReturnValue(mockChild as any); + + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + MockGetSessionOptions.mockReturnValue({ + sessionId: 'test-session-id', + channelName: 'test-channel' + } as any); + + // Mock IPC responses - check the actual message type + MockAwaitIpc.mockImplementation(async (child: any, matcher: any): Promise => { + // Test the matcher with a sample message to determine type + const testHello: IpcResponse = { t: 'hello:ack', id: 'mock-id' }; + const testLoad: IpcResponse = { t: 'load:ack', id: 'mock-id', warnings: [], errors: [] }; + const testManifest: IpcResponse = { t: 'manifest:result', id: 'mock-id', tools: [] }; + + if (matcher(testHello)) { + return testHello; + } + if (matcher(testLoad)) { + return testLoad; + } + if (matcher(testManifest)) { + return { + t: 'manifest:result', + id: 'mock-id', + tools: [ + { + id: 'tool-1', + name: 'Tool1', + description: 'Tool 1', + inputSchema: {} + } + ] + } as IpcResponse; + } + throw new Error('Unexpected matcher'); + }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it.each([ + { + description: 'with empty toolModules', + toolModules: [], + expectedBuiltinOnly: true + }, + { + description: 'with undefined toolModules', + toolModules: undefined, + expectedBuiltinOnly: true + }, + { + description: 'with null toolModules', + toolModules: null, + expectedBuiltinOnly: true + } + ])('should return only built-in tools, $description', async ({ toolModules }) => { + MockGetOptions.mockReturnValue({ + toolModules, + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost + } as any); + + const result = await composeTools(builtinTools); + + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBeGreaterThan(0); + expect(MockSpawn).not.toHaveBeenCalled(); + }); + + it.each([ + { + description: 'Node 20', + nodeVersion: 20, + toolModules: ['./module.js'] + }, + { + description: 'Node 21', + nodeVersion: 21, + toolModules: ['./module.js'] + }, + { + description: 'Node 18', + nodeVersion: 18, + toolModules: ['./module.js'] + } + ])('should skip externals and warn when Node < 22, $description', async ({ nodeVersion, toolModules }) => { + MockGetOptions.mockReturnValue({ + toolModules, + nodeVersion, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost + } as any); + + const result = await composeTools(builtinTools); + + expect(Array.isArray(result)).toBe(true); + // Note: File resolution happens before Node version check in normalizeTools. + // If the file path is invalid, it will log a file resolution error. + // If the file path is valid, it will be added to filePackageEntries and then + // the Node version check will log the warning. + // Since './module.js' is likely invalid in the test environment, we expect + // a file resolution error rather than the Node version warning. + expect(MockLog.warn).toHaveBeenCalled(); + // The warning should be either the Node version check or a file resolution error + const warnCalls = MockLog.warn.mock.calls.flat(); + const hasExpectedWarning = warnCalls.some((msg: unknown) => + typeof msg === 'string' && ( + msg.includes('External tool plugins require Node >= 22') || + msg.includes('Failed to resolve file path') + )); + + expect(hasExpectedWarning).toBe(true); + expect(MockSpawn).not.toHaveBeenCalled(); + }); + + it('should spawn tools host and return built-in + proxy creators', async () => { + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + const result = await composeTools(builtinTools); + + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBeGreaterThan(0); + // Verify result includes built-in tools plus proxy tools + expect(result.length).toBeGreaterThanOrEqual(builtinTools.length); + // Indirect evidence via non-empty result beyond built-ins is sufficient + }); + + it('should handle spawn errors gracefully', async () => { + MockSpawn.mockImplementationOnce(() => { + throw new Error('Spawn failed'); + }); + + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + const result = await composeTools(builtinTools); + + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBeGreaterThan(0); + expect(MockLog.warn.mock.calls).toMatchSnapshot('warn'); + }); + + it('should handle IPC errors gracefully', async () => { + MockAwaitIpc.mockRejectedValueOnce(new Error('IPC timeout')); + + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + const result = await composeTools(builtinTools); + + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBeGreaterThan(0); + expect(MockLog.warn.mock.calls).toMatchSnapshot('warn'); + }); + + it('should use strict isolation when pluginIsolation is strict', async () => { + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: 'strict' + } as any); + + await composeTools(builtinTools); + + // Strict isolation should not throw; behavior verified by successful composition + expect(true).toBe(true); + }); + + it('should send hello, load, and manifest requests', async () => { + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + const result = await composeTools(builtinTools); + + // Successful composition implies IPC handshake succeeded + // Verify result includes tools + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBeGreaterThan(0); + }); + + it('should log warnings and errors from load', async () => { + // Mock load:ack response with warnings and errors + MockAwaitIpc.mockImplementation(async (child: any, matcher: any): Promise => { + const testHello: IpcResponse = { t: 'hello:ack', id: 'mock-id' }; + const testLoad: IpcResponse = { + t: 'load:ack', + id: 'mock-id', + warnings: ['Warning 1', 'Warning 2'], + errors: ['Error 1'] + }; + const testManifest: IpcResponse = { t: 'manifest:result', id: 'mock-id', tools: [] }; + + if (matcher(testHello)) { + return testHello; + } + if (matcher(testLoad)) { + return testLoad; + } + if (matcher(testManifest)) { + return testManifest; + } + throw new Error('Unexpected matcher'); + }); + + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + await composeTools(builtinTools); + + // Verify warnings and errors were logged + expect(MockLog.warn.mock.calls).toMatchSnapshot('warn'); + }); + + it('should clean up host on child exit', async () => { + // Create a fresh mock child for this test to track calls + const testMockChild = { + ...mockChild, + once: jest.fn() + }; + + // Override the spawn mock to return our test child for this test + MockSpawn.mockReturnValueOnce(testMockChild as any); + + MockGetOptions.mockReturnValue({ + toolModules: ['./test-module.js'], + nodeVersion: 22, + contextPath: '/test/path', + contextUrl: 'file:///test/path', + pluginHost: DEFAULT_OPTIONS.pluginHost, + pluginIsolation: undefined + } as any); + + await composeTools(builtinTools); + + // Cleanup handlers registration is internal; ensure no exceptions occurred during composition + expect(Array.isArray(MockSpawn.mock.calls)).toBe(true); + }); +}); +*/ diff --git a/src/__tests__/server.toolsUser.test.ts b/src/__tests__/server.toolsUser.test.ts new file mode 100644 index 0000000..61d2a19 --- /dev/null +++ b/src/__tests__/server.toolsUser.test.ts @@ -0,0 +1,610 @@ +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('should use memoization consistently for contextPath and contextUrl results', () => { + const config = './fixtures/tool.mjs'; + + 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.toEqual(resultOne); + expect(resultThree).toEqual(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.tools.ts b/src/server.tools.ts new file mode 100644 index 0000000..e4bd08f --- /dev/null +++ b/src/server.tools.ts @@ -0,0 +1,570 @@ +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; + +/** + * 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 }); + + 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. + * @returns Updated array of normalized tool modules + */ +const getFilePackageToolModules = ({ contextPath, contextUrl, toolModules }: GlobalOptions = getOptions()): string[] => + normalizeTools + .memo(toolModules, { contextPath, contextUrl }) + .filter(tool => tool.type === 'file' || tool.type === 'package') + .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; + + // 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), + pluginHost.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. + * @returns {Promise} Promise that resolves when the host is stopped or noop. + */ +const sendToolsHostShutdown = async ( + { pluginHost }: GlobalOptions = getOptions(), + { sessionId }: AppSession = getSessionOptions() +): Promise => { + const handle = activeHostsBySession.get(sessionId); + + if (!handle) { + return; + } + + const gracePeriodMs = (Number.isInteger(pluginHost?.gracePeriodMs) && pluginHost.gracePeriodMs) || 0; + const fallbackGracePeriodMs = gracePeriodMs + 200; + + const child = handle.child; + let resolved = false; + let forceKillPrimary: NodeJS.Timeout | undefined; + let forceKillFallback: NodeJS.Timeout | undefined; + + await new Promise(resolve => { + const resolveOnce = () => { + if (resolved) { + return; + } + + resolved = true; + child.off('exit', resolveOnce); + child.off('disconnect', resolveOnce); + + if (forceKillPrimary) { + clearTimeout(forceKillPrimary); + } + + if (forceKillFallback) { + clearTimeout(forceKillFallback); + } + + try { + handle.closeStderr?.(); + } catch {} + + activeHostsBySession.delete(sessionId); + resolve(); + }; + + try { + send(child, { t: 'shutdown', id: makeId() }); + } catch {} + + const shutdownChild = () => { + try { + if (!child?.killed) { + child.kill('SIGKILL'); + } + } finally { + resolveOnce(); + } + }; + + // Primary grace period + forceKillPrimary = setTimeout(shutdownChild, gracePeriodMs); + forceKillPrimary?.unref?.(); + + // Fallback grace period + forceKillFallback = setTimeout(shutdownChild, fallbackGracePeriodMs); + forceKillFallback?.unref?.(); + + child.once('exit', resolveOnce); + child.once('disconnect', resolveOnce); + }); +}; + +/** + * 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 result: McpToolCreator[] = [...builtinCreators]; + const usedNames = new Set(builtinCreators.map(creator => getBuiltInToolName(creator)).filter(Boolean) as string[]); + + if (!Array.isArray(toolModules) || toolModules.length === 0) { + return result; + } + + const filePackageEntries: NormalizedToolEntry[] = []; + const tools = normalizeTools.memo(toolModules, { contextUrl, contextPath }); + + tools.forEach(tool => { + switch (tool.type) { + case 'file': + case 'package': + filePackageEntries.push(tool); + break; + case 'invalid': + log.warn(tool.error); + break; + case 'tuple': + case 'object': + case 'creator': { + const toolName = 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).`); + break; + } + + if (toolName) { + usedNames.add(toolName); + } + + result.push(tool.value as McpToolCreator); + break; + } + } + }); + + // Load file-based via Tools Host (Node.js version gate applies here) + if (filePackageEntries.length === 0) { + return result; + } + + if (nodeVersion < 22) { + log.warn('External tool plugins require Node >= 22; skipping file-based tools.'); + + return result; + } + + try { + const host = await spawnToolsHost(); + + // Filter manifest by reserved names BEFORE proxying + const filteredTools = host.tools.filter(tool => { + if (usedNames.has(tool.name)) { + log.warn(`Skipping plugin tool "${tool.name}" – name already used by built-in/inline tool.`); + + return false; + } + usedNames.add(tool.name); + + return true; + }); + + const filteredHandle = { ...host, tools: filteredTools } as HostHandle; + const proxies = makeProxyCreators(filteredHandle); + + // Associate the spawned host with the current session + activeHostsBySession.set(sessionId, host); + + // Clean up on exit or disconnect + const onChildExitOrDisconnect = () => { + const current = activeHostsBySession.get(sessionId); + + if (current && current.child === host.child) { + try { + host.closeStderr?.(); + } catch {} + activeHostsBySession.delete(sessionId); + } + host.child.off('exit', onChildExitOrDisconnect); + host.child.off('disconnect', onChildExitOrDisconnect); + }; + + try { + host.child.once('exit', onChildExitOrDisconnect); + host.child.once('disconnect', onChildExitOrDisconnect); + } catch {} + + return [...result, ...proxies]; + } catch (error) { + log.warn('Failed to start Tools Host; skipping externals and continuing with built-ins/inline.'); + log.warn(formatUnknownError(error)); + + return result; + } +}; + +export { + composeTools, + computeFsReadAllowlist, + debugChild, + getBuiltInToolName, + getFilePackageToolModules, + logWarningsErrors, + makeProxyCreators, + sendToolsHostShutdown, + spawnToolsHost +}; diff --git a/src/server.toolsUser.ts b/src/server.toolsUser.ts new file mode 100644 index 0000000..f4b5755 --- /dev/null +++ b/src/server.toolsUser.ts @@ -0,0 +1,714 @@ +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { dirname, extname, isAbsolute, resolve } from 'node:path'; +import { isPlainObject } 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']); + +/** + * Return an object key value. + * + * @param obj + * @param key + */ +const sanitizeDataProp = (obj: unknown, key: string) => { + 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 => + JSON.stringify([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 +};