From 2df9accc18ddd70f76cf380ef3ca17312f136a1f Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 6 Aug 2026 01:38:08 -0400 Subject: [PATCH] feat(collections): pf-4401 process spawn, ipc * collection.patternFlyDocs, Schemas, add collectionName prop * options.collections, alias toolOptions * options.defaults, collectionModules base option * server.collections, expand funcs for spawning children * server.collectionsHost, handlers, load external collections * server.collectionsHostCreator, child-process guards, helpers for creators * server.collectionsIpc, parent-child comm layer * server.collectionsUser, helpers base for collections-as-plugins * server.tools, avoid process registry naming collisions with collections * server, graceful shutdown for collections process --- package.json | 1 + .../options.defaults.test.ts.snap | 1 + .../server.collections.test.ts.snap | 392 ++++++++++++ .../server.collectionsHost.test.ts.snap | 151 +++++ .../server.collectionsUser.test.ts.snap | 82 +++ .../__snapshots__/server.test.ts.snap | 33 + src/__tests__/server.collections.test.ts | 563 +++++++++++++++++- src/__tests__/server.collectionsHost.test.ts | 209 +++++++ .../server.collectionsHostCreator.test.ts | 161 +++++ src/__tests__/server.collectionsIpc.test.ts | 13 + src/__tests__/server.collectionsUser.test.ts | 142 +++++ src/__tests__/server.tools.test.ts | 5 +- src/collection.patternFlyDocs.ts | 5 + src/collection.patternFlySchemas.ts | 5 + src/options.collections.ts | 17 + src/options.defaults.ts | 5 + src/server.collections.ts | 449 +++++++++++++- src/server.collectionsHost.ts | 263 ++++++++ src/server.collectionsHostCreator.ts | 135 +++++ src/server.collectionsIpc.ts | 90 +++ src/server.collectionsUser.ts | 222 +++++++ src/server.tools.ts | 14 +- src/server.ts | 8 +- .../__snapshots__/stdioTransport.test.ts.snap | 2 + 24 files changed, 2932 insertions(+), 36 deletions(-) create mode 100644 src/__tests__/__snapshots__/server.collectionsHost.test.ts.snap create mode 100644 src/__tests__/__snapshots__/server.collectionsUser.test.ts.snap create mode 100644 src/__tests__/server.collectionsHost.test.ts create mode 100644 src/__tests__/server.collectionsHostCreator.test.ts create mode 100644 src/__tests__/server.collectionsIpc.test.ts create mode 100644 src/__tests__/server.collectionsUser.test.ts create mode 100644 src/options.collections.ts create mode 100644 src/server.collectionsHost.ts create mode 100644 src/server.collectionsHostCreator.ts create mode 100644 src/server.collectionsIpc.ts create mode 100644 src/server.collectionsUser.ts diff --git a/package.json b/package.json index 3ca6fbb0..a949d6cb 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "type": "module", "imports": { "~docsCatalog": "./src/docs.json", + "#collectionsHost": "./dist/server.collectionsHost.js", "#toolsHost": "./dist/server.toolsHost.js" }, "exports": { diff --git a/src/__tests__/__snapshots__/options.defaults.test.ts.snap b/src/__tests__/__snapshots__/options.defaults.test.ts.snap index d784cfbe..200927e3 100644 --- a/src/__tests__/__snapshots__/options.defaults.test.ts.snap +++ b/src/__tests__/__snapshots__/options.defaults.test.ts.snap @@ -2,6 +2,7 @@ exports[`options defaults should return specific properties: defaults 1`] = ` { + "collectionModules": [], "contextManagement": false, "contextPath": "/", "contextUrl": "file:///", diff --git a/src/__tests__/__snapshots__/server.collections.test.ts.snap b/src/__tests__/__snapshots__/server.collections.test.ts.snap index 8b0f464e..358a7667 100644 --- a/src/__tests__/__snapshots__/server.collections.test.ts.snap +++ b/src/__tests__/__snapshots__/server.collections.test.ts.snap @@ -1,5 +1,41 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +exports[`composeCollections should attempt to setup creators, default package creators 1`] = ` +{ + "collectionsCount": 2, + "log": [], +} +`; + +exports[`composeCollections should attempt to setup creators, inline and case-variant duplicate creators 1`] = ` +{ + "collectionsCount": 3, + "log": [ + [ + "Skipping collection plugin "LOREM" – name already used by built-in/inline collection.", + ], + ], +} +`; + +exports[`composeCollections should attempt to setup creators, inline and duplicate creators 1`] = ` +{ + "collectionsCount": 3, + "log": [ + [ + "Skipping collection plugin "lorem" – name already used by built-in/inline collection.", + ], + ], +} +`; + +exports[`composeCollections should attempt to setup creators, inline creators 1`] = ` +{ + "collectionsCount": 3, + "log": [], +} +`; + exports[`composeCollections should match snapshot for composed collection creators 1`] = ` [ { @@ -11,3 +47,359 @@ exports[`composeCollections should match snapshot for composed collection creato }, ] `; + +exports[`composeCollections should return default creators on collections host error 1`] = ` +{ + "collectionsCount": 2, + "log": [ + [ + "Failed to start Collections Host; skipping hosted collections and continuing with built-in/inline collections. Error: Mock spawn failure", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, access denied 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, access denied, alt messaging 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_ACCESS_DENIED]: fs.readFileSync access is denied by permission model: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet", + ], + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, access denied, multiple lines 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet", + ], + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_ACCESS_DENIED]: Access denied: FileSystemRead, resource: /lorem/ipsum/dolor/sit/amet", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, default 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] lorem ipsum dolor sit amet", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, empty string 1`] = ` +{ + "debug": [], +} +`; + +exports[`debugChild should format and forward lines to log.debug, generic multiline error 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Lorem ipsum", + ], + [ + "[collections-host pid=123 sid=1234567890] dolor sit", + ], + [ + "[collections-host pid=123 sid=1234567890] amet", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, generic multiline error with spaces 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Lorem ipsum", + ], + [ + "[collections-host pid=123 sid=1234567890] dolor sit", + ], + [ + "[collections-host pid=123 sid=1234567890] amet", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, module not found 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lorem/ipsum/dolor/sit/amet' imported from /test/path", + ], + ], +} +`; + +exports[`debugChild should format and forward lines to log.debug, module not found, multiple lines 1`] = ` +{ + "debug": [ + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lorem/ipsum/dolor/sit/amet' imported from /test/path", + ], + [ + "[collections-host pid=123 sid=1234567890] Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lorem/ipsum/dolor/sit/amet' imported from /test/path", + ], + ], +} +`; + +exports[`getBuiltInCollectionNames should log a warning when a collection name does not exist: warning 1`] = ` +[ + [ + "Built-in collection at index 0 is missing the static name property, "collectionName"", + ], +] +`; + +exports[`logWarningsErrors should log warnings and errors, with both warnings and errors 1`] = ` +{ + "errors": [ + [ + "Collections load errors (1) + - Error 1", + ], + ], + "warnings": [ + [ + "Collections load warnings (1) + - Warning 1", + ], + ], +} +`; + +exports[`logWarningsErrors should log warnings and errors, with empty arrays 1`] = ` +{ + "errors": [], + "warnings": [], +} +`; + +exports[`logWarningsErrors should log warnings and errors, with errors only 1`] = ` +{ + "errors": [ + [ + "Collections load errors (2) + - Error 1 + - Error 2", + ], + ], + "warnings": [], +} +`; + +exports[`logWarningsErrors should log warnings and errors, with single error 1`] = ` +{ + "errors": [ + [ + "Collections load errors (1) + - Single error", + ], + ], + "warnings": [], +} +`; + +exports[`logWarningsErrors should log warnings and errors, with single warning 1`] = ` +{ + "errors": [], + "warnings": [ + [ + "Collections load warnings (1) + - Single warning", + ], + ], +} +`; + +exports[`logWarningsErrors should log warnings and errors, with undefined warnings and errors 1`] = ` +{ + "errors": [], + "warnings": [], +} +`; + +exports[`logWarningsErrors should log warnings and errors, with warnings only 1`] = ` +{ + "errors": [], + "warnings": [ + [ + "Collections 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: request 1`] = ` +[ + [ + { + "args": { + "loremIpsum": 7, + }, + "collectionId": "loremIpsum", + "t": "invoke", + }, + "invoke:result", + 10, + ], +] +`; + +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: request 1`] = ` +[ + [ + { + "args": { + "loremIpsum": 7, + }, + "collectionId": "loremIpsum", + "t": "invoke", + }, + "invoke:result", + 10, + ], +] +`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false: handler 1`] = `[Error: Collection invocation failed]`; + +exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false: request 1`] = ` +[ + [ + { + "args": { + "loremIpsum": 7, + }, + "collectionId": "loremIpsum", + "t": "invoke", + }, + "invoke:result", + 10, + ], +] +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per collection, multiple collections 1`] = ` +{ + "debug": [], + "output": [ + [ + "Lorem Ipsum", + [Function], + ], + [ + "Dolor Sit", + [Function], + ], + ], +} +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per collection, no collections 1`] = ` +{ + "debug": [], + "output": [], +} +`; + +exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per collection, single collection 1`] = ` +{ + "debug": [], + "output": [ + [ + "Lorem Ipsum", + [Function], + ], + ], +} +`; + +exports[`spawnCollectionHost attempt to spawn the Collections Host, with no pluginIsolation, node 24: spawn 1`] = ` +{ + "spawnConfig": { + "enableStderrDebug": [Function], + "importSpecifier": "#collectionsHost", + "isolation": { + "fsReadAllowlist": [], + "mode": "none", + "nodeVersion": 24, + }, + "label": "Collections Host", + }, +} +`; + +exports[`spawnCollectionHost attempt to spawn the Collections Host, with strict pluginIsolation, node 22: spawn 1`] = ` +{ + "spawnConfig": { + "enableStderrDebug": [Function], + "importSpecifier": "#collectionsHost", + "isolation": { + "fsReadAllowlist": [], + "mode": "strict", + "nodeVersion": 22, + }, + "label": "Collections Host", + }, +} +`; + +exports[`spawnCollectionHost attempt to spawn the Collections Host, with strict pluginIsolation, node 24: spawn 1`] = ` +{ + "spawnConfig": { + "enableStderrDebug": [Function], + "importSpecifier": "#collectionsHost", + "isolation": { + "fsReadAllowlist": [], + "mode": "strict", + "nodeVersion": 24, + }, + "label": "Collections Host", + }, +} +`; + +exports[`spawnCollectionHost attempt to spawn the Collections Host, with undefined pluginIsolation, node 22: spawn 1`] = ` +{ + "spawnConfig": { + "enableStderrDebug": [Function], + "importSpecifier": "#collectionsHost", + "isolation": { + "fsReadAllowlist": [], + "mode": "none", + "nodeVersion": 22, + }, + "label": "Collections Host", + }, +} +`; diff --git a/src/__tests__/__snapshots__/server.collectionsHost.test.ts.snap b/src/__tests__/__snapshots__/server.collectionsHost.test.ts.snap new file mode 100644 index 00000000..bbb4f79c --- /dev/null +++ b/src/__tests__/__snapshots__/server.collectionsHost.test.ts.snap @@ -0,0 +1,151 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`requestInvoke should attempt collection invocation, handler attempting to return a DOMException-like object, with name, message and multiline line stack 1`] = ` +{ + "error": "Internal error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler attempting to return a browser-like ErrorEvent-like object, with name, message and multiline line stack 1`] = ` +{ + "error": "Internal error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler attempting to return an error-like object, with message 1`] = ` +{ + "id": "request-id", + "ok": true, + "result": { + "message": "Handler error", + }, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler attempting to return an error-like object, with name and multiline line stack 1`] = ` +{ + "error": "Internal error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler attempting to return an error-like object, with name and single line stack 1`] = ` +{ + "error": "Internal error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler attempting to return an error-like object, with single line stack 1`] = ` +{ + "id": "request-id", + "ok": true, + "result": { + "message": "Handler error", + "stack": "Stack trace", + }, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler returning AggregateError 1`] = ` +{ + "error": "Internal error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler returning error 1`] = ` +{ + "error": "Internal error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler returning null 1`] = ` +{ + "id": "request-id", + "ok": true, + "result": null, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler returning promise 1`] = ` +{ + "id": "request-id", + "ok": true, + "result": { + "data": "async-result", + }, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler returning undefined 1`] = ` +{ + "id": "request-id", + "ok": true, + "result": undefined, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, handler throwing error 1`] = ` +{ + "error": "Handler error", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, mismatched state and request collection IDs 1`] = ` +{ + "error": "Unknown collectionId", + "id": "request-id", + "ok": false, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should attempt collection invocation, successful handler 1`] = ` +{ + "id": "request-id", + "ok": true, + "result": { + "data": "result", + }, + "t": "invoke:result", +} +`; + +exports[`requestInvoke should timeout when handler takes too long 1`] = ` +[ + [ + { + "error": { + "message": "Invoke timeout", + }, + "id": "request-id", + "ok": false, + "t": "invoke:result", + }, + ], +] +`; diff --git a/src/__tests__/__snapshots__/server.collectionsUser.test.ts.snap b/src/__tests__/__snapshots__/server.collectionsUser.test.ts.snap new file mode 100644 index 00000000..2ee2ae39 --- /dev/null +++ b/src/__tests__/__snapshots__/server.collectionsUser.test.ts.snap @@ -0,0 +1,82 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`normalizeCollections should normalize configs, array of tuples 1`] = ` +[ + { + "collectionName": "loremIpsum", + "error": undefined, + "index": 0, + "type": "tuple", + }, + { + "collectionName": "dolorSit", + "error": undefined, + "index": 1, + "type": "tuple", + }, +] +`; + +exports[`normalizeCollections should normalize configs, mix of non-configs 1`] = ` +[ + { + "collectionName": undefined, + "error": "createMcpCollection: invalid configuration used at index 0: Unsupported type object", + "index": 0, + "type": "invalid", + }, + { + "collectionName": undefined, + "error": "createMcpCollection: invalid configuration used at index 1: Unsupported type undefined", + "index": 1, + "type": "invalid", + }, + { + "collectionName": undefined, + "error": "createMcpCollection: invalid configuration used at index 2: Unsupported type object", + "index": 2, + "type": "invalid", + }, + { + "collectionName": undefined, + "error": "createMcpCollection: invalid configuration used at index 3: Unsupported type object", + "index": 3, + "type": "invalid", + }, +] +`; + +exports[`normalizeCollections should normalize configs, single tuple 1`] = ` +[ + { + "collectionName": "loremIpsum", + "error": undefined, + "index": 0, + "type": "tuple", + }, +] +`; + +exports[`normalizeTuple should normalize the config, basic 1`] = ` +{ + "collectionName": "loremIpsum", + "original": [ + "loremIpsum", + "[MockFunction]", + ], + "type": "tuple", + "value": "[CollectionCreator]", +} +`; + +exports[`normalizeTuple should normalize the config, untrimmed name, async handler 1`] = ` +{ + "collectionName": "loremIpsum", + "original": [ + "loremIpsum ", + "[MockFunction]", + ], + "type": "tuple", + "value": "[CollectionCreator]", +} +`; diff --git a/src/__tests__/__snapshots__/server.test.ts.snap b/src/__tests__/__snapshots__/server.test.ts.snap index 6ffe41b4..b14a77fe 100644 --- a/src/__tests__/__snapshots__/server.test.ts.snap +++ b/src/__tests__/__snapshots__/server.test.ts.snap @@ -9,6 +9,9 @@ exports[`runServer should allow server to be stopped, http stop server: diagnost [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -71,6 +74,9 @@ exports[`runServer should allow server to be stopped, stdio stop server: diagnos [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -133,6 +139,9 @@ exports[`runServer should attempt to run server, create transport, connect, and [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -206,6 +215,9 @@ exports[`runServer should attempt to run server, disable SIGINT handler: diagnos [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -274,6 +286,9 @@ exports[`runServer should attempt to run server, enable SIGINT handler explicitl [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -347,6 +362,9 @@ exports[`runServer should attempt to run server, log warnings for experimental o [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -435,6 +453,9 @@ exports[`runServer should attempt to run server, register a tool: diagnostics 1` [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -516,6 +537,9 @@ exports[`runServer should attempt to run server, register multiple tools: diagno [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -604,6 +628,9 @@ exports[`runServer should attempt to run server, use custom options: diagnostics [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -677,6 +704,9 @@ exports[`runServer should attempt to run server, use default tools, http: diagno [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], @@ -759,6 +789,9 @@ exports[`runServer should attempt to run server, use default tools, stdio: diagn [ "Server stats enabled.", ], + [ + "No external collections loaded.", + ], [ "No external resources loaded.", ], diff --git a/src/__tests__/server.collections.test.ts b/src/__tests__/server.collections.test.ts index c4fd399f..2d289139 100644 --- a/src/__tests__/server.collections.test.ts +++ b/src/__tests__/server.collections.test.ts @@ -1,14 +1,440 @@ -import { composeCollections } from '../server.collections'; +import { + composeCollections, + computeFsReadAllowlist, + debugChild, + getBuiltInCollectionNames, + logWarningsErrors, + makeProxyCreators, + secureBuiltinCreators, + sendCollectionsHostShutdown, + spawnCollectionHost +} from '../server.collections'; +import { log } from '../logger'; import { getOptions, getSessionOptions } from '../options.context'; +import { spawnChildProcess, shutdownChildProcess, activeChildrenBySession } from '../server.process'; + +jest.mock('../server.process', () => ({ + spawnChildProcess: jest.fn(), + shutdownChildProcess: jest.fn().mockResolvedValue(undefined), + activeChildrenBySession: new Map() +})); jest.mock('../options.context', () => ({ - getOptions: jest.fn(), - getSessionOptions: jest.fn() + getOptions: jest.fn(() => ({})), + getSessionOptions: jest.fn(() => ({ sessionId: 'test' })), + getLoggerOptions: 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)) })); +describe('getBuiltInCollectionNames', () => { + const MockLog = jest.mocked(log); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return built-in collection name', () => { + const collectionName = 'loremIpsum'; + const creator = () => {}; + + creator.collectionName = collectionName; + + expect(getBuiltInCollectionNames([creator] as any).has(collectionName.toLowerCase())).toBe(true); + }); + + it('should log a warning when a collection name does not exist', () => { + const creator = () => {}; + + getBuiltInCollectionNames([creator] as any); + + expect(MockLog.warn.mock.calls).toMatchSnapshot('warning'); + }); +}); + +describe('secureBuiltinCreators', () => { + it('should wrap builtin creators and set _isInternal: true', () => { + const mockHandler = jest.fn(); + const mockCreator: any = jest.fn(() => ['test-collection', mockHandler, { isRequired: true }]); + + mockCreator.collectionName = 'test-collection'; + + const [secured]: any[] = secureBuiltinCreators([mockCreator]); + const [name, callback, config] = secured({}); + + expect(name).toBe('test-collection'); + expect(config?._isInternal).toBe(true); + expect(config?.isRequired).toBe(true); + expect(callback).toBe(mockHandler); + expect(getBuiltInCollectionNames([secured] as any).has('test-collection')).toBe(true); + }); +}); + +describe('computeFsReadAllowlist', () => { + it.each([ + { + description: 'with contextPath', + options: { + contextPath: '/' + }, + expected: ['/'] + }, + { + description: 'with missing context path', + options: { + contextPath: undefined + }, + expected: [] + } + ])('should return a list of allowed paths, $description', ({ options, expected }) => { + expect(computeFsReadAllowlist(options as any)).toEqual(expected); + }); +}); + +describe('logWarningsErrors', () => { + const MockLog = jest.mocked(log); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'with warnings only', + warnings: ['Warning 1', 'Warning 2'], + errors: [] + }, + { + description: 'with errors only', + warnings: [], + errors: ['Error 1', 'Error 2'] + }, + { + description: 'with both warnings and errors', + warnings: ['Warning 1'], + errors: ['Error 1'] + }, + { + description: 'with empty arrays', + warnings: [], + errors: [] + }, + { + description: 'with undefined warnings and errors', + warnings: undefined, + errors: undefined + }, + { + description: 'with single warning', + warnings: ['Single warning'], + errors: [] + }, + { + description: 'with single error', + warnings: [], + errors: ['Single error'] + } + ])('should log warnings and errors, $description', ({ warnings, errors }) => { + logWarningsErrors({ warnings, errors } as any); + + expect({ + warnings: MockLog.warn.mock.calls, + errors: MockLog.error.mock.calls + }).toMatchSnapshot(); + }); +}); + +describe('debugChild', () => { + let debugSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + debugSpy = jest.spyOn(log, 'debug').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 format and forward lines to log.debug, $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({ + debug: debugSpy.mock.calls + }).toMatchSnapshot(); + + unsubscribe(); + expect(mockOff).toHaveBeenCalledWith('data', mockHandler); + }); +}); + +describe('spawnCollectionHost', () => { + const MockSpawnChildProcess = jest.mocked(spawnChildProcess); + + 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 Collections Host, $description', async ({ options }) => { + const updatedOptions = { collectionModules: [], pluginHost: { loadTimeoutMs: 10, invokeTimeoutMs: 10 }, ...options }; + const mockPid = 123; + const mockCollections = [{ name: 'alphaCollection', id: 'alphaCollection' }]; + const mockRequest = jest.fn() + .mockResolvedValueOnce({ t: 'hello:ack', id: 'id-1' }) + .mockResolvedValueOnce({ t: 'load:ack', id: 'id-1', warnings: [], errors: [] }) + .mockResolvedValueOnce({ t: 'manifest:result', id: 'id-1', collections: mockCollections }); + + MockSpawnChildProcess.mockReturnValue({ + child: { pid: mockPid } as any, + request: mockRequest, + closeStderr: jest.fn() + } as any); + + jest.mocked(getOptions).mockReturnValue(updatedOptions as any); + + const result = await spawnCollectionHost(updatedOptions as any); + + expect(result.child.pid).toBe(mockPid); + expect(result.collections).toEqual(mockCollections); + expect(mockRequest).toHaveBeenCalledTimes(3); + + expect({ + spawnConfig: MockSpawnChildProcess.mock.calls?.[0]?.[0] + }).toMatchSnapshot('spawn'); + }); + + it('should throw when spawn fails', async () => { + jest.mocked(getOptions).mockReturnValue({ collectionModules: [], pluginHost: {} } as any); + MockSpawnChildProcess.mockImplementationOnce(() => { + throw new Error('Failed to resolve Collections Host entry \'#collectionsHost\'.'); + }); + + await expect( + spawnCollectionHost({ nodeVersion: 24, pluginIsolation: 'strict', pluginHost: {} } as any) + ).rejects.toThrow(/Failed to resolve Collections Host/); + }); +}); + +describe('makeProxyCreators', () => { + const MockLog = jest.mocked(log); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'no collections', + collections: [] + }, + { + description: 'single collection', + collections: [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum' + } + ] + }, + { + description: 'multiple collections', + collections: [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum' + }, + { + id: 'dolorSit', + name: 'Dolor Sit' + } + ] + } + ])('should attempt to return proxy creators, a function wrapper per collection, $description', ({ collections }) => { + const proxies = makeProxyCreators({ collections } as any, { pluginHost: { invokeTimeoutMs: 10 } } as any); + const output = proxies.map(proxy => { + const [name, handler] = proxy(); + + return [ + name, + 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 collections = [ + { + id: 'loremIpsum', + name: 'Lorem Ipsum' + } + ]; + + const mockRequest = jest.fn().mockResolvedValueOnce({ t: 'invoke:result', ...response }); + const mockHandle = { collections, request: mockRequest, child: { pid: 123 } }; + + const proxies = makeProxyCreators(mockHandle as any, { pluginHost: { invokeTimeoutMs: 10 } } as any); + const proxyCreator = proxies[0]; + + expect(proxyCreator).toBeDefined(); + + const [, handler] = proxyCreator!(); + + await expect(handler({ loremIpsum: 7 })).rejects.toMatchSnapshot('handler'); + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest.mock.calls).toMatchSnapshot('request'); + }); +}); + +describe('sendCollectionsHostShutdown', () => { + const MockShutdownChildProcess = jest.mocked(shutdownChildProcess); + + beforeEach(() => { + jest.clearAllMocks(); + activeChildrenBySession.clear(); + }); + + it('should attempt graceful shutdown of child', async () => { + const child = { pid: 123 }; + const handle = { child, closeStderr: jest.fn() }; + const sessionId = 'test-session-id'; + const registryKey = `${sessionId}:collections`; + + activeChildrenBySession.set(registryKey, handle as any); + + await sendCollectionsHostShutdown({ pluginHost: { gracePeriodMs: 10 } } as any, { sessionId } as any); + + expect(MockShutdownChildProcess).toHaveBeenCalledTimes(1); + expect(MockShutdownChildProcess).toHaveBeenCalledWith(handle, { + gracePeriodMs: 10, + sessionId: registryKey, + label: 'Collections Host' + }); + }); +}); + describe('composeCollections', () => { + const MockSpawnChildProcess = jest.mocked(spawnChildProcess); + const MockLog = jest.mocked(log); + const MockGetOptions = jest.mocked(getOptions); + const MockGetSessionOptions = jest.mocked(getSessionOptions); + + // Mock default creators + const loremIpsum = () => ['loremIpsum', () => {}, { isRequired: true }]; + const dolorSitAmet = () => ['dolorSitAmet', () => {}, { isRequired: false }]; + const consecteturAdipiscingElit: any = () => ['consecteturAdipiscingElit', () => {}, { runInChildProcess: true }]; + + loremIpsum.collectionName = 'loremIpsum'; + dolorSitAmet.collectionName = 'dolorSitAmet'; + consecteturAdipiscingElit.collectionName = 'consecteturAdipiscingElit'; + beforeEach(() => { jest.clearAllMocks(); + activeChildrenBySession.clear(); + MockGetOptions.mockReturnValue({ collectionModules: [], pluginHost: { loadTimeoutMs: 10, invokeTimeoutMs: 10 } } as any); + MockGetSessionOptions.mockReturnValue({ sessionId: 'test-session-id' } as any); }); it('should wrap builtin creators and set _isInternal: true', async () => { @@ -41,8 +467,8 @@ describe('composeCollections', () => { session: { sessionId: '123' } } ])('should handle various configurations, $description', async ({ options, session }) => { - (getOptions as jest.Mock).mockReturnValue(options); - (getSessionOptions as jest.Mock).mockReturnValue(session); + MockGetOptions.mockReturnValue(options as any); + MockGetSessionOptions.mockReturnValue(session as any); const mockCreator: any = jest.fn(() => ['test', jest.fn()]); const result: any = await composeCollections([mockCreator], options as any, session as any); @@ -63,4 +489,131 @@ describe('composeCollections', () => { expect(output).toMatchSnapshot(); }); + + it.each([ + { + description: 'default package creators', + nodeVersion: 22, + modules: [], + expectedModuleCount: 2 + }, + { + description: 'inline creators', + nodeVersion: 22, + modules: [ + ['lorem', () => {}] + ], + expectedModuleCount: 3 + }, + { + description: 'inline and duplicate creators', + nodeVersion: 22, + modules: [ + ['lorem', () => {}], + ['lorem', () => {}] + ], + expectedModuleCount: 3 + }, + { + description: 'inline and case-variant duplicate creators', + nodeVersion: 22, + modules: [ + ['lorem', () => {}], + ['LOREM', () => {}] + ], + expectedModuleCount: 3 + } + ])('should attempt to setup creators, $description', async ({ modules, nodeVersion, expectedModuleCount }) => { + const mockChild = { + pid: 123, + once: jest.fn(), + off: jest.fn() + }; + const mockHostedCollections = modules.map(([name]) => ({ name, id: name })); + + const sessionId = 'test-session-id'; + + const mockRequest = jest.fn() + .mockResolvedValueOnce({ t: 'hello:ack', id: 'id-1' }) + .mockResolvedValueOnce({ t: 'load:ack', id: 'id-1', warnings: [], errors: [] }) + .mockResolvedValueOnce({ t: 'manifest:result', id: 'id-1', collections: mockHostedCollections }); + + MockSpawnChildProcess.mockReturnValue({ + child: mockChild as any, + request: mockRequest, + closeStderr: jest.fn() + } as any); + + const defaultCreators: any[] = [loremIpsum, dolorSitAmet, consecteturAdipiscingElit]; + const globalOptions: any = { collectionModules: modules, nodeVersion, contextUrl: 'file:///test/path', contextPath: '/test/path' }; + const sessionOptions: any = { sessionId }; + + MockGetOptions.mockReturnValue(globalOptions); + + const collections = await composeCollections(defaultCreators, globalOptions, sessionOptions); + + expect(collections.length).toBe(expectedModuleCount); + expect({ + collectionsCount: collections.length, + log: MockLog.warn.mock.calls + }).toMatchSnapshot(); + }); + + it('should attempt to setup handlers for child exit, disconnect', async () => { + const onceHandlers: Record = {}; + const mockChild = { + pid: 123, + once: jest.fn((event: string, handler: any) => { + onceHandlers[event] = handler; + }), + off: jest.fn() + }; + const sessionId = 'test-session-id'; + + const mockRequest = jest.fn() + .mockResolvedValueOnce({ t: 'hello:ack', id: 'id-1' }) + .mockResolvedValueOnce({ t: 'load:ack', id: 'id-1', warnings: [], errors: [] }) + .mockResolvedValueOnce({ t: 'manifest:result', id: 'id-1', collections: [] }); + + MockSpawnChildProcess.mockReturnValue({ + child: mockChild as any, + request: mockRequest, + closeStderr: jest.fn() + } as any); + + const defaultCreators: any[] = [loremIpsum, dolorSitAmet, consecteturAdipiscingElit]; + const globalOptions: any = { collectionModules: [], nodeVersion: 22, contextUrl: 'file:///test/path', contextPath: '/test/path' }; + const sessionOptions: any = { sessionId }; + + MockGetOptions.mockReturnValue(globalOptions); + + await composeCollections(defaultCreators, globalOptions, sessionOptions); + + if (onceHandlers['disconnect']) { + onceHandlers['disconnect'](); + } + + expect(mockChild.once).toHaveBeenCalledTimes(2); + expect(mockChild.off).toHaveBeenCalledWith('exit', onceHandlers['exit']); + expect(mockChild.off).toHaveBeenCalledWith('disconnect', onceHandlers['disconnect']); + }); + + it('should return default creators on collections host error', async () => { + const sessionId = 'test-session-id'; + + MockSpawnChildProcess.mockImplementationOnce(() => { + throw new Error('Mock spawn failure'); + }); + + const defaultCreators: any[] = [loremIpsum, dolorSitAmet, consecteturAdipiscingElit]; + const globalOptions: any = { collectionModules: [], nodeVersion: 22, contextUrl: 'file:///test/path', contextPath: '/test/path' }; + const sessionOptions: any = { sessionId }; + const collections = await composeCollections(defaultCreators, globalOptions, sessionOptions); + + expect(collections.length).toBe(2); + expect({ + collectionsCount: collections.length, + log: MockLog.warn.mock.calls + }).toMatchSnapshot(); + }); }); diff --git a/src/__tests__/server.collectionsHost.test.ts b/src/__tests__/server.collectionsHost.test.ts new file mode 100644 index 00000000..6d82825f --- /dev/null +++ b/src/__tests__/server.collectionsHost.test.ts @@ -0,0 +1,209 @@ +import { requestInvoke, createCollectionsHost } from '../server.collectionsHost'; + +describe('requestInvoke', () => { + let mockSend: jest.Mock; + + beforeEach(() => { + mockSend = jest.fn(); + process.send = mockSend; + }); + + afterEach(() => { + delete (process as any).send; + jest.clearAllMocks(); + }); + + it.each([ + { + description: 'successful handler', + handlerResult: { data: 'result' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler returning promise', + handlerResult: Promise.resolve({ data: 'async-result' }), + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler throwing error', + handlerResult: Promise.reject(new Error('Handler error')), + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler returning error', + handlerResult: new Error('Handler error'), + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'mismatched state and request collection IDs', + handlerResult: { data: 'result' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-2' + }, + { + description: 'handler returning AggregateError', + handlerResult: new AggregateError(['Handler error']), + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler attempting to return an error-like object, with message', + handlerResult: { message: 'Handler error' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler attempting to return an error-like object, with single line stack', + handlerResult: { message: 'Handler error', stack: 'Stack trace' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler attempting to return an error-like object, with name and single line stack', + handlerResult: { name: 'Mock ERROR', message: 'Handler error', stack: 'Stack trace' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler attempting to return an error-like object, with name and multiline line stack', + handlerResult: { name: 'Mock', message: 'Handler error', stack: 'Stack trace\nSecond line' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler attempting to return a DOMException-like object, with name, message and multiline line stack', + handlerResult: { name: 'DOMException', message: 'Handler error', stack: 'DOMException: message\n at line x' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler attempting to return a browser-like ErrorEvent-like object, with name, message and multiline line stack', + handlerResult: { name: 'ErrorEvent', message: 'Handler error', stack: 'ErrorEvent: message\n at line x' }, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler returning undefined', + handlerResult: undefined, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + }, + { + description: 'handler returning null', + handlerResult: null, + stateCollectionId: 'coll-1', + requestCollectionId: 'coll-1' + } + ])('should attempt collection invocation, $description', async ({ handlerResult, stateCollectionId, requestCollectionId }) => { + const mockState = { + collectionMap: new Map(), + descriptors: [ + { + id: stateCollectionId, + name: 'CollectionName', + source: 'module1' + } + ], + invokeTimeoutMs: 1000 + }; + + mockState.collectionMap.set( + stateCollectionId, + [ + 'CollectionName', + jest.fn().mockImplementation(async () => handlerResult), + { runInChildProcess: true } + ] + ); + + const ctx = { send: jest.fn() }; + + const promise = requestInvoke(mockState as any, { t: 'invoke', id: 'request-id', collectionId: requestCollectionId, args: { param: 'value' } }, ctx); + + await promise; + + expect(ctx.send.mock.calls.length).toBe(1); + + const { error, ...rest } = ctx.send.mock.calls[0][0]; + + expect({ + ...((error?.message && { error: error?.message }) || undefined), + ...rest + }).toMatchSnapshot(); + }); + + it('should timeout when handler takes too long', async () => { + jest.useFakeTimers(); + + const stateCollectionId = 'coll-1'; + const requestCollectionId = 'coll-1'; + const mockState = { + collectionMap: new Map(), + descriptors: [ + { + id: stateCollectionId, + name: 'CollectionName', + source: 'module1' + } + ], + invokeTimeoutMs: 100 + }; + + // Create a handler that resolves after timeout would fire + const handler = jest.fn(() => new Promise(resolve => { + setTimeout(resolve, 101); + })); + + mockState.collectionMap.set( + stateCollectionId, + [ + 'CollectionName', + handler, + { runInChildProcess: true } + ] + ); + + const ctx = { send: jest.fn() }; + + const invokePromise = requestInvoke(mockState as any, { t: 'invoke', id: 'request-id', collectionId: requestCollectionId, args: {} }, ctx); + + // Wait for handler to be called, timeout to be set up + await Promise.resolve(); + + // Advance timers past timeout + jest.advanceTimersByTime(102); + + // Wait for the timeout message to be sent + await Promise.resolve(); + + // Verify timeout message was sent + expect(ctx.send.mock.calls).toMatchSnapshot(); + + // Wait for the function to complete + await invokePromise; + + expect(ctx.send).toHaveBeenCalledTimes(1); + + jest.useRealTimers(); + }); +}); + +describe('createCollectionsHost', () => { + it('should route load then reply with load:ack via the host', async () => { + const sendSpy = jest.fn(); + + (process as any).send = sendSpy; + + const { bootstrapMessage } = createCollectionsHost(); + + await bootstrapMessage({ t: 'load', id: 'L1', specs: [] } as any); + + expect(sendSpy).toHaveBeenCalledWith( + expect.objectContaining({ t: 'load:ack', id: 'L1', warnings: [], errors: [] }) + ); + }); +}); diff --git a/src/__tests__/server.collectionsHostCreator.test.ts b/src/__tests__/server.collectionsHostCreator.test.ts new file mode 100644 index 00000000..78f2d676 --- /dev/null +++ b/src/__tests__/server.collectionsHostCreator.test.ts @@ -0,0 +1,161 @@ +import { resolveCreators } from '../server.collectionsHostCreator'; + +describe('resolveCreators', () => { + it('should return a normalized module output with expected properties', () => { + const mockHandler = jest.fn(); + const moduleExport = { + default: () => ['Collection1', mockHandler, { runInChildProcess: true }] + }; + + const [result] = resolveCreators(moduleExport); + const [name, handler, config]: any[] = result?.() || []; + + expect([ + name, + handler, + config + ]).toEqual([ + 'Collection1', + mockHandler, + { runInChildProcess: true } + ]); + }); + + it.each([ + { + description: 'valid format, default export with function, tuple', + moduleExports: { + default: () => ['Collection1', jest.fn(), { runInChildProcess: true }] + }, + isValid: true + }, + { + description: 'valid format, default export with function, array of functions with tuple return', + moduleExports: { + default: () => [ + () => ['Collection1', jest.fn(), { runInChildProcess: true }], + () => ['Collection1', jest.fn(), { runInChildProcess: true }] + ] + }, + isValid: true + }, + { + description: 'valid format, default export with array of functions with tuple return', + moduleExports: { + default: [ + () => ['Collection1', jest.fn(), { runInChildProcess: true }], + () => ['Collection1', jest.fn(), { runInChildProcess: true }] + ] + }, + isValid: true + }, + { + description: 'invalid format, default export with function, array of tuples', + moduleExports: { + default: () => [ + ['Collection1', jest.fn(), { runInChildProcess: true }], + ['Collection2', jest.fn(), { runInChildProcess: true }] + ] + }, + isValid: false + }, + { + description: 'invalid format, default export with tuple', + moduleExports: { + default: ['Collection1', jest.fn(), { runInChildProcess: true }] + }, + isValid: false + }, + { + description: 'invalid format, default export with array of tuples', + moduleExports: { + default: [ + ['Collection1', jest.fn(), { runInChildProcess: true }], + ['Collection2', jest.fn(), { runInChildProcess: true }] + ] + }, + isValid: false + }, + { + description: 'invalid format, default export function that returns empty', + moduleExports: { + default: () => {} + }, + isValid: false + }, + { + description: 'invalid format, empty module', + moduleExports: {}, + isValid: false + }, + { + description: 'invalid format, default export function that returns null', + moduleExports: { + default: () => null + }, + isValid: false + }, + { + description: 'invalid format, null', + moduleExports: null, + isValid: false + }, + { + description: 'invalid format, default export function that returns undefined', + moduleExports: { + default: () => undefined + }, + isValid: false + }, + { + description: 'invalid format, undefined', + moduleExports: undefined, + isValid: false + }, + { + description: 'invalid format, default export function that throws', + moduleExports: { + default: () => { + throw new Error('Function error'); + } + }, + isValid: false + }, + { + description: 'invalid format, function that throws', + moduleExports: () => { + throw new Error('Function error'); + }, + isValid: false + }, + { + description: 'invalid format, function returning unsupported shape', + moduleExports: () => 'not a collection or creators[]', + isValid: false + }, + { + description: 'invalid format, array with non-function elements', + moduleExports: ['not a function', 123, {}], + isValid: false + }, + { + description: 'invalid format, named exports only', + moduleExports: { + named1: () => ['Collection1', jest.fn(), { runInChildProcess: true }], + named2: () => ['Collection2', jest.fn(), { runInChildProcess: true }] + }, + isValid: false + } + ])('should normalize module exports with specific formats, $description', ({ moduleExports, isValid }) => { + const result = resolveCreators(moduleExports); + + expect(Array.isArray(result)).toBe(true); + expect(result.length > 0).toBe(isValid); + }); + + it('should throw an error on empty if throwOnEmpty option is enabled', () => { + expect(() => { + resolveCreators({}, undefined, { throwOnEmpty: true }); + }).toThrow(/No usable collection creators found/); + }); +}); diff --git a/src/__tests__/server.collectionsIpc.test.ts b/src/__tests__/server.collectionsIpc.test.ts new file mode 100644 index 00000000..caf4dda5 --- /dev/null +++ b/src/__tests__/server.collectionsIpc.test.ts @@ -0,0 +1,13 @@ +import { send, awaitIpc, makeId, matchResponse, serializeError } from '../server.collectionsIpc'; + +describe('server.collectionsIpc re-exports', () => { + it.each([ + ['send', send], + ['awaitIpc', awaitIpc], + ['makeId', makeId], + ['matchResponse', matchResponse], + ['serializeError', serializeError] + ])('should re-export %s from server.processIpc', (_name, fn) => { + expect(typeof fn).toBe('function'); + }); +}); diff --git a/src/__tests__/server.collectionsUser.test.ts b/src/__tests__/server.collectionsUser.test.ts new file mode 100644 index 00000000..2c29ace5 --- /dev/null +++ b/src/__tests__/server.collectionsUser.test.ts @@ -0,0 +1,142 @@ +import { normalizeTuple, normalizeCollections, sanitizeStaticCollectionName } from '../server.collectionsUser'; + +describe('sanitizeStaticCollectionName', () => { + it('should return the trimmed name when collectionName is set with defineProperty', () => { + const func = function testCreator() {}; + + Object.defineProperty(func, 'collectionName', { value: ' MyCollection ', writable: false, enumerable: false, configurable: false }); + + expect(sanitizeStaticCollectionName(func)).toBe('MyCollection'); + }); + + it('should return undefined when collectionName is defined through a "getter"', () => { + const func = function testCreator() {}; + + Object.defineProperty(func, 'collectionName', { + get() { + throw new Error('should not be called'); + } + }); + + expect(sanitizeStaticCollectionName(func)).toBeUndefined(); + }); + + it('should return undefined when a Proxy throws', () => { + const target = function testCreator() {}; + const proxy = new Proxy(target, { + getOwnPropertyDescriptor(_target, prop) { + if (prop === 'collectionName') { + throw new Error('trap'); + } + + return Reflect.getOwnPropertyDescriptor(_target, prop as PropertyKey); + } + }); + + expect(sanitizeStaticCollectionName(proxy)).toBeUndefined(); + }); +}); + +describe('normalizeTuple', () => { + it.each([ + { + description: 'basic', + tuple: ['loremIpsum', () => {}] + }, + { + description: 'untrimmed name, async handler', + tuple: ['loremIpsum ', async () => {}] + }, + { + description: 'missing handler', + tuple: ['dolorSit'] + }, + { + description: 'undefined', + tuple: undefined + }, + { + description: 'null', + tuple: null + } + ])('should normalize the config, $description', ({ tuple }) => { + const updated = normalizeTuple(tuple); + + if (updated) { + // Check the creator output + const creator = updated.value as (...args: any[]) => any; + + const result = creator(); + + expect(result).toEqual([ + updated.collectionName, + expect.any(Function), + { runInChildProcess: true, isInternal: false } + ]); + + // Strip function from snapshots/assertions + const original = updated.original as any; + + if (Array.isArray(original) && typeof original[1] === 'function') { + original[1] = '[MockFunction]'; + } + + expect({ + ...updated, + value: '[CollectionCreator]' + }).toMatchSnapshot(); + } else { + expect(updated).toBeUndefined(); + } + }); + + it('should have a memo property', () => { + expect(normalizeTuple.memo).toBeDefined(); + }); +}); + +describe('normalizeCollections', () => { + it.each([ + { + description: 'single tuple', + config: ['loremIpsum', () => {}] + }, + { + description: 'array of tuples', + config: [ + ['loremIpsum', () => {}], + ['dolorSit', async () => {}] + ] + }, + { + description: 'mix of non-configs', + config: [null, undefined, { x: 1 }, new Error('lorem ipsum')] + } + ])('should normalize configs, $description', ({ config }) => { + const result = normalizeCollections(config); + const configLength = !normalizeTuple(config) && Array.isArray(config) ? config.length : 1; + + expect(result.length).toBe(configLength); + expect(result.map(({ index, type, collectionName, error }) => ({ index, type, collectionName, error }))).toMatchSnapshot(); + }); + + it('should flatten when using non-tuple configs (arrays)', () => { + const config = [[1, 2, 3], ['lorem', 'ipsum', 'dolor', 'sit']]; + const result = normalizeCollections(config); + const configLength = config.flat().length; + + expect(result.length).toBe(configLength); + }); + + it('should have a memo property', () => { + expect(normalizeCollections.memo).toBeDefined(); + }); + + it('should handle memoization context isolation', () => { + const config = ['a', () => {}]; + const resultOne = normalizeCollections.memo(config); + const resultTwo = normalizeCollections.memo(config); + + expect(resultTwo).toEqual(resultOne); + }); +}); diff --git a/src/__tests__/server.tools.test.ts b/src/__tests__/server.tools.test.ts index d36013de..3b02bbc2 100644 --- a/src/__tests__/server.tools.test.ts +++ b/src/__tests__/server.tools.test.ts @@ -474,15 +474,16 @@ describe('sendToolsHostShutdown', () => { const child = { pid: 123 }; const handle = { child, closeStderr: jest.fn() }; const sessionId = 'test-session-id'; + const registryKey = `${sessionId}:tools`; - activeChildrenBySession.set(sessionId, handle as any); + activeChildrenBySession.set(registryKey, handle as any); await sendToolsHostShutdown({ pluginHost: { gracePeriodMs: 10 } } as any, { sessionId } as any); expect(MockShutdownChildProcess).toHaveBeenCalledTimes(1); expect(MockShutdownChildProcess).toHaveBeenCalledWith(handle, { gracePeriodMs: 10, - sessionId, + sessionId: registryKey, label: 'Tools Host' }); }); diff --git a/src/collection.patternFlyDocs.ts b/src/collection.patternFlyDocs.ts index 5812ac0c..84c0f1d1 100644 --- a/src/collection.patternFlyDocs.ts +++ b/src/collection.patternFlyDocs.ts @@ -66,4 +66,9 @@ const patternFlyDocsCollection = (): McpCollection => { ]; }; +/** + * A collection name, typically the first entry in the tuple. Used in logging and deduplication. + */ +patternFlyDocsCollection.collectionName = 'patternfly-docs'; + export { patternFlyDocsCollection }; diff --git a/src/collection.patternFlySchemas.ts b/src/collection.patternFlySchemas.ts index 56d26b93..d737fabc 100644 --- a/src/collection.patternFlySchemas.ts +++ b/src/collection.patternFlySchemas.ts @@ -77,6 +77,11 @@ const patternFlySchemasCollection = (): McpCollection => { ]; }; +/** + * A collection name, typically the first entry in the tuple. Used in logging and deduplication. + */ +patternFlySchemasCollection.collectionName = 'patternfly-component-schemas'; + export { patternFlySchemasCollection }; diff --git a/src/options.collections.ts b/src/options.collections.ts new file mode 100644 index 00000000..ecbb98be --- /dev/null +++ b/src/options.collections.ts @@ -0,0 +1,17 @@ +import { type ToolOptions, setToolOptions } from './options.tools'; + +/** + * Options for records. A limited subset of options. + * + * @alias ToolOptions + */ +type CollectionOptions = ToolOptions; + +/** + * Return a refined set of options from global options for records. + * + * @alias setToolOptions + */ +const setCollectionOptions = setToolOptions; + +export { setCollectionOptions, type CollectionOptions }; diff --git a/src/options.defaults.ts b/src/options.defaults.ts index 3ee44588..2f38c09c 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -2,6 +2,7 @@ import { basename, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; import packageJson from '../package.json'; import { type ToolModule } from './server.toolsUser'; +import { type CollectionModule } from './server.collectionsUser'; import { getNodeMajorVersion } from './options.helpers'; /** @@ -47,6 +48,8 @@ import { getNodeMajorVersion } from './options.helpers'; * @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 {CollectionModule|CollectionModule[]} collectionModules - Array of external collection modules + * (ESM specs or paths) to be loaded and registered with the server. * @property urlRegex - Regular expression pattern for URL matching. * @property version - Version of the package. * @property whitelist - Central outbound-URL policy options. @@ -83,6 +86,7 @@ interface DefaultOptions { stats: StatsOptions; toolMemoOptions: Partial; toolModules: ToolModule | ToolModule[]; + collectionModules: CollectionModule | CollectionModule[]; urlRegex: RegExp; version: string; whitelist: WhitelistOptions; @@ -560,6 +564,7 @@ const DEFAULT_OPTIONS: DefaultOptions = { resourceModules: [], toolMemoOptions: TOOL_MEMO_OPTIONS, toolModules: [], + collectionModules: [], separator: DEFAULT_SEPARATOR, urlRegex: URL_REGEX, version: (process.env.NODE_ENV === 'local' && '0.0.0') || packageJson.version, diff --git a/src/server.collections.ts b/src/server.collections.ts index 32540522..2c2b3042 100644 --- a/src/server.collections.ts +++ b/src/server.collections.ts @@ -1,41 +1,448 @@ +import { type ChildProcess } from 'node:child_process'; import { type AppSession, type GlobalOptions } from './options'; +import { formatUnknownError, log } from './logger'; +import { + spawnChildProcess, + shutdownChildProcess, + activeChildrenBySession, + type ChildHandle +} from './server.process'; import { getOptions, getSessionOptions } from './options.context'; -import { type McpCollectionCreator } from './collections'; +import { type McpCollectionCreator, type McpCollectionResult } from './collections'; +import { setCollectionOptions } from './options.collections'; +import { type CollectionDescriptor, type IpcResponse } from './server.collectionsIpc'; +import { + normalizeCollections, + sanitizeStaticCollectionName, + type NormalizedCollectionEntry +} from './server.collectionsUser'; +import { applyStaticProperty } from './server.processUser'; + +/** + * Handle for a spawned Host process. + * + * @property collections - Array of collection descriptors. + */ +type HostHandle = ChildHandle & { + collections: CollectionDescriptor[]; +}; + +/** + * Compute the allowlist for the Host. + * + * @param {GlobalOptions} options - Global options. + * @returns Array of absolute directories to allow read access. + */ +const computeFsReadAllowlist = ({ contextPath }: GlobalOptions = getOptions()): string[] => { + const directories = new Set(); + + if (contextPath) { + directories.add(contextPath); + } + + return [...directories]; +}; + +/** + * Get a set of collection names from the builtin creators. + * + * @param builtinCreators - Array of builtin collection creators + * @returns Set of collection names + */ +const getBuiltInCollectionNames = (builtinCreators: McpCollectionCreator[]) => + new Set(builtinCreators.map((creator, index) => { + const builtInCollectionName = sanitizeStaticCollectionName(creator)?.toLowerCase?.(); + + if (!builtInCollectionName) { + log.warn(`Built-in collection at index ${index} is missing the static name property, "collectionName"`); + } + + return builtInCollectionName; + }).filter(Boolean) as string[]); + +/** + * Wrap built-in creators to set a trusted `_isInternal: true` status. + * + * @param builtinCreators - Array of builtin collection creators + * @returns Array of secured builtin collection creators + */ +const secureBuiltinCreators = (builtinCreators: McpCollectionCreator[]) => + builtinCreators.map((creator, index): McpCollectionCreator => { + const secured: McpCollectionCreator = opt => { + const [name, callback, config] = creator(opt); + + return [ + name, + callback, + { + ...config, + _isInternal: true + } + ]; + }; + + const collectionName = sanitizeStaticCollectionName(creator); + + if (collectionName) { + applyStaticProperty('collectionName', collectionName, secured); + } else { + log.warn( + `Built-in collection at index ${index} is missing the static name property, "collectionName"` + ); + } + + return secured; + }); + +/** + * 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(`Collections load warnings (${warnings.length})\n${lines.join('\n')}`); + } + + if (Array.isArray(errors) && errors.length > 0) { + const lines = errors.map(error => ` - ${String(error)}`); + + log.error(`Collections load errors (${errors.length})\n${lines.join('\n')}`); + } +}; + +/** + * Get normalized "inline" modules. Inline modules can be internal or embedded and are explicitly trusted. + * + * @param {GlobalOptions} options - Global options. + * @param options.collectionModules - Array of modules to normalize + * @returns - Filtered array of normalized "inline" tool modules + */ +const getInlineCollections = ({ collectionModules }: GlobalOptions = getOptions()): NormalizedCollectionEntry[] => + normalizeCollections.memo(collectionModules).filter(module => module.type === 'tuple'); + +/** + * Get normalized "inline" modules. + * + * @param {GlobalOptions} options - Global options. + * @param options.collectionModules - Array of modules to normalize + * @returns - Filtered array of normalized "inline" tool modules + */ +const getInvalidCollections = ({ collectionModules }: GlobalOptions = getOptions()): NormalizedCollectionEntry[] => + normalizeCollections.memo(collectionModules).filter(module => module.type === 'invalid'); + +/** + * 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 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 = `[collections-host pid=${childPid} sid=${sessionId}] ${line}`; + + // Default: debug-level passthrough + log.debug(tagged); + } + }; + + child.stderr?.on?.('data', debugHandler); + + return () => { + child.stderr?.off?.('data', debugHandler); + }; +}; + +/** + * Spawn the Collections Host (child process), load external collections, and return a host handle. + * + * @note The load IPC payload uses a generic `options` field (not `collectionOptions`) so creator + * options, and related session context, can share a host-agnostic shape. Tools still pass + * domain-specific `toolOptions`. Future iterations should align Tools Host IPC to this generic + * `options` contract. + * + * @param {GlobalOptions} options - Global options. + * @returns Host handle used by `makeProxyCreators` and shutdown. + * + * @throws {Error} If the Collections Host entry `#collectionsHost` cannot be resolved, or if the child + * process fails to spawn or respond during the handshake within the configured timeout. + */ +const spawnCollectionHost = async ( + options: GlobalOptions = getOptions() +): Promise => { + const { pluginIsolation, pluginHost, nodeVersion } = options || {}; + const { loadTimeoutMs, invokeTimeoutMs } = pluginHost || {}; + const collectionOptions = setCollectionOptions(options); + + const handle = spawnChildProcess({ + importSpecifier: '#collectionsHost', + label: 'Collections Host', + isolation: { + mode: pluginIsolation === 'strict' ? 'strict' : 'none', + nodeVersion, + fsReadAllowlist: computeFsReadAllowlist() + }, + enableStderrDebug: child => debugChild(child) + }); + + // hello + await handle.request({ t: 'hello' }, 'hello:ack', loadTimeoutMs); + + // load + const loadAck = await handle.request>( + { t: 'load', specs: [], invokeTimeoutMs, options: collectionOptions }, + 'load:ack', + loadTimeoutMs + ); + + logWarningsErrors(loadAck); + + // manifest + const manifest = await handle.request>( + { t: 'manifest:get' }, + 'manifest:result', + loadTimeoutMs + ); + + return { ...handle, collections: manifest.collections as CollectionDescriptor[] }; +}; + +/** + * Recreate parent-side creators that forward invocations to the Host. + * + * @param {HostHandle} handle - Host handle. + * @param {GlobalOptions} options - Global options. + * @returns Array of creators + */ +const makeProxyCreators = ( + handle: HostHandle, + { pluginHost }: GlobalOptions = getOptions() +): McpCollectionCreator[] => handle.collections.map((collection): McpCollectionCreator => () => { + const name = collection.name; + const invokeTimeoutMs = Math.max(0, Number(pluginHost?.invokeTimeoutMs) || 0); + + const handler = async (args?: unknown): Promise => { + const response = await handle.request>( + { t: 'invoke', collectionId: collection.id, args }, + 'invoke:result', + invokeTimeoutMs + ); + + if ('ok' in response && response.ok === false) { + const invocationError = new Error(response.error?.message || 'Collection 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; + } + + const errorCause = response.error?.cause as { details?: unknown } | undefined; + + invocationError.details = response.error?.details || errorCause?.details; + throw invocationError; + } + + return response.result as McpCollectionResult; + }; + + return [name, handler]; +}); + +/** + * Best-effort Tools Host shutdown for the current session. + * + * Policy: + * - Primary grace defaults to 0 ms (internal-only, from DEFAULT_OPTIONS.pluginHost.gracePeriodMs) + * - Single fallback kill at grace + 200 ms to avoid racing simultaneous kills + * - Close logging for child(ren) stderr + * + * @param {GlobalOptions} options - Global options. + * @param {AppSession} sessionOptions - Session options. + */ +const sendCollectionsHostShutdown = async ( + { pluginHost }: GlobalOptions = getOptions(), + { sessionId }: AppSession = getSessionOptions() +): Promise => { + const registryKey = `${sessionId}:collections`; + const handle = activeChildrenBySession.get(registryKey) as HostHandle | undefined; + + await shutdownChildProcess(handle, { + gracePeriodMs: Math.max(0, Number(pluginHost?.gracePeriodMs) || 0), + sessionId: registryKey, + label: 'Collections Host' + }); +}; /** * Composes multi-source record collections across process boundaries. * * @param builtinCreators - * @param {GlobalOptions} _options - Global options. - * @param {AppSession} _session - Session options. + * @param {GlobalOptions} options - Global options. + * @param {AppSession} session - Session options. * @returns Promise array of collection creators. */ const composeCollections = async ( builtinCreators: McpCollectionCreator[], - _options: GlobalOptions = getOptions(), - _session: AppSession = getSessionOptions() + options: GlobalOptions = getOptions(), + session: AppSession = getSessionOptions() ): Promise => { - // Wrap built-in creators to enforce trusted _isInternal. Ties into what options, session values are available. - const securedBuiltinCreators = builtinCreators.map((creator): McpCollectionCreator => opt => { - const [name, callback, config] = creator(opt); - - return [ - name, - callback, - { - ...config, - _isInternal: true - } - ]; - }); + const { collectionModules, nodeVersion, contextUrl, contextPath } = options; + const { sessionId } = session; + const registryKey = `${sessionId}:collections`; + const existingSession = activeChildrenBySession.get(registryKey); + + if (existingSession) { + log.warn(`Existing Collections Host session detected ${sessionId}. Shutting down the existing host before creating a new one.`); + await sendCollectionsHostShutdown(); + } + + // Intercept and wrap built-in creators to enforce trusted isInternal: true status + const securedBuiltinCreators = secureBuiltinCreators(builtinCreators); + + const updatedCollectionModules = Array.isArray(collectionModules) ? collectionModules : []; + const usedNames = getBuiltInCollectionNames(securedBuiltinCreators); + + if (updatedCollectionModules.length === 0) { + log.info('No external collections loaded.'); + } - if (securedBuiltinCreators.length === 0) { + if (updatedCollectionModules.length === 0 && securedBuiltinCreators.length === 0) { return []; } - return securedBuiltinCreators; + // Temporary placeholder for collections-as-plugins + const filePackageCreators: NormalizedCollectionEntry[] = []; + const invalidCreators = getInvalidCollections({ collectionModules, contextUrl, contextPath } as GlobalOptions); + const inlineCreators: NormalizedCollectionEntry[] = getInlineCollections({ collectionModules, contextUrl, contextPath } as GlobalOptions); + + const normalizeCollectionName = (collectionName?: string) => collectionName?.trim?.()?.toLowerCase?.(); + + invalidCreators.forEach(({ error }) => { + log.warn(error); + }); + + const localCreators: McpCollectionCreator[] = []; + const hostedCreators: McpCollectionCreator[] = []; + + for (const creator of securedBuiltinCreators) { + const [, , config] = creator(options); + const runHost = typeof config?.runInChildProcess === 'function' + ? await config.runInChildProcess(options) + : Boolean(config?.runInChildProcess); + + if (runHost) { + hostedCreators.push(creator); + } else { + localCreators.push(creator); + } + } + + const filteredInlineCreators = inlineCreators.map(collection => + collection.value as McpCollectionCreator).filter(Boolean); + + hostedCreators.push(...filteredInlineCreators); + + if (filePackageCreators.length && (!nodeVersion || nodeVersion < 22)) { + log.warn('External collection plugins require Node >= 22; skipping file-based collections.'); + } + + if (hostedCreators.length === 0) { + return localCreators; + } + + let host: HostHandle | undefined; + + // Clean up on exit or disconnect + const onChildExitOrDisconnect = () => { + if (!host) { + return; + } + + const current = activeChildrenBySession.get(registryKey); + + if (current && current.child === host.child) { + try { + host.closeStderr(); + log.info('Collections Host stderr reader closed.'); + } catch (error) { + log.error(`Failed to close Collections Host stderr reader: ${formatUnknownError(error)}`); + } + + activeChildrenBySession.delete(registryKey); + } + + host.child.off('exit', onChildExitOrDisconnect); + host.child.off('disconnect', onChildExitOrDisconnect); + }; + + try { + host = await spawnCollectionHost(options); + + // Filter manifest by reserved names BEFORE proxying + const filteredCollections = host.collections.filter(collection => { + const collectionName = normalizeCollectionName(collection.name); + + if (collectionName && usedNames.has(collectionName)) { + log.warn(`Skipping collection plugin "${collection.name}" – name already used by built-in/inline collection.`); + + return false; + } + + if (collectionName) { + usedNames.add(collectionName); + } + + return true; + }); + + const filteredHandle = { ...host, collections: filteredCollections } as HostHandle; + const proxiedCreators = makeProxyCreators(filteredHandle); + + activeChildrenBySession.set(registryKey, host); + + host.child.once('exit', onChildExitOrDisconnect); + host.child.once('disconnect', onChildExitOrDisconnect); + + return [...localCreators, ...proxiedCreators]; + } catch (error) { + log.warn(`Failed to start Collections Host; skipping hosted collections and continuing with built-in/inline collections. ${formatUnknownError(error)}`); + + return localCreators; + } }; export { - composeCollections + composeCollections, + computeFsReadAllowlist, + debugChild, + getBuiltInCollectionNames, + getInlineCollections, + getInvalidCollections, + logWarningsErrors, + makeProxyCreators, + secureBuiltinCreators, + sendCollectionsHostShutdown, + spawnCollectionHost }; diff --git a/src/server.collectionsHost.ts b/src/server.collectionsHost.ts new file mode 100644 index 00000000..382d12d6 --- /dev/null +++ b/src/server.collectionsHost.ts @@ -0,0 +1,263 @@ +import { + type IpcRequest, + type CollectionDescriptor, + makeId +} from './server.collectionsIpc'; +import { serializeError, type SerializedError } from './server.processIpc'; +import { createProcessHost, type HostContext } from './server.processHost'; +import { DEFAULT_OPTIONS } from './options.defaults'; +import { type McpCollectionCreator, type McpCollection } from './collections'; +import { resolveCreators } from './server.collectionsHostCreator'; +import { type CollectionOptions } from './options.collections'; + +/** + * SubType of IpcRequest for "load" requests. + */ +type LoadRequest = Extract; + +/** + * SubType of IpcRequest for "invoke" requests. + */ +type InvokeRequest = Extract; + +/** + * State object for the collections host. + */ +type HostState = { + collectionMap: Map; + descriptors: CollectionDescriptor[]; + invokeTimeoutMs: number; +}; + +/** + * Create a new host state object. + * + * @param invokeTimeoutMs + * @returns {HostState} + */ +const createHostState = (invokeTimeoutMs = DEFAULT_OPTIONS.pluginHost.invokeTimeoutMs): HostState => ({ + collectionMap: new Map(), + descriptors: [], + invokeTimeoutMs +}); + +/** + * Check if a value is an error or an error-like object. + * + * Handles cross-realm Error detection via tag checks for `[object Error]`, `[object AggregateError]`, + * and `[object DOMException]`. Does not treat `[object ErrorEvent]` as error-like in the + * Node context; add if your runtime can emit `ErrorEvent`. + * + * @param value + * @returns True if the value is an error-like object, false otherwise. + */ +const isErrorLike = (value: unknown) => { + if (!value || (typeof value !== 'object' && typeof value !== 'function')) { + return false; + } + + if (value instanceof Error || value instanceof AggregateError) { + return true; + } + + const tag = Object.prototype.toString.call(value); + + if (tag === '[object Error]' || tag === '[object AggregateError]' || tag === '[object DOMException]') { + return true; + } + + const val = value as Record; + const has = (key: string) => + Object.hasOwn(val, key) && typeof val[key] === 'string' && val[key].length > 0; + + if (!has('message')) { + return false; + } + + const isNameLike = has('name') && (val.name as string).toLowerCase().endsWith('error'); + const isStackLike = has('stack') && (val.stack as string).includes('\n'); + + return isNameLike || isStackLike; +}; + +/** + * Load external creators, realize them, and normalize `inputSchema` in the child. + * + * Stores the real Zod schema in memory for runtime validation and sends a JSON-safe schema in descriptors. + * + * @param {LoadRequest} request - Load request object. + * @returns New state object with updated collection/descriptors and warnings/errors. + */ +const performLoad = async (request: LoadRequest): Promise => { + const nextInvokeTimeout = typeof request?.invokeTimeoutMs === 'number' && Number.isFinite(request.invokeTimeoutMs) && request.invokeTimeoutMs > 0 + ? request.invokeTimeoutMs + : DEFAULT_OPTIONS.pluginHost.invokeTimeoutMs; + + const state = createHostState(nextInvokeTimeout); + const warnings: string[] = []; + const errors: string[] = []; + const options: CollectionOptions | undefined = request.options; + let module: unknown; + + for (const spec of request.specs || []) { + // Import the module. On fail, move to the next module. + try { + const dynamicImport = new Function('spec', 'return import(spec)') as (spec: string) => Promise; + + // let's export a common "collection" function for records/collections + module = await dynamicImport(spec); + } catch (error) { + errors.push(`Failed import: ${spec}: ${String((error as Error)?.message || error)}`); + continue; + } + + // Does the module export a creator function? On fail, move to the next module. + let creators: McpCollectionCreator[] = []; + + try { + creators = resolveCreators(module, options, { throwOnEmpty: true }); + } catch (error) { + warnings.push(`No usable creators in module ${spec}: ${String((error as Error)?.message || error)}`); + continue; + } + + // Finally, convert to JSON for manifest, store, push descriptor + for (const creator of creators) { + try { + const create = creator as (opts?: unknown) => McpCollection; + const collection = create(options); + + const collectionId = makeId(); + + state.collectionMap.set(collectionId, collection); + state.descriptors.push({ + id: collectionId, + name: collection[0], + source: spec + }); + } catch (error) { + warnings.push(`Collection creator threw while realizing: ${spec}: ${String((error as Error)?.message || error)}`); + } + } + } + + return { ...state, warnings, errors }; +}; + +/** + * Invoke a realized collection by id. Validates arguments against the in-memory Zod schema. + * + * @example + * // On validation failure, returns + * { ok: false, error: { code: 'INVALID_ARGS', details } } + * + * @param {HostState} state + * @param {InvokeRequest} request + * @param {HostContext} ctx + */ +const requestInvoke = async (state: HostState, request: InvokeRequest, ctx: HostContext) => { + const collection = state.collectionMap.get(request.collectionId); + + if (!collection) { + ctx.send({ + t: 'invoke:result', + id: request.id, + ok: false, + error: { message: 'Unknown collectionId' } + }); + + return; + } + + let settled = false; + + const timer = setTimeout(() => { + if (settled) { + return; + } + + settled = true; + + ctx.send({ + t: 'invoke:result', + id: request.id, + ok: false, + error: { message: 'Invoke timeout' } + }); + }, state.invokeTimeoutMs); + + timer?.unref?.(); + + const handler = collection[1]; + + try { + // Child-side validation + const updatedRequestArgs = request.args; + + // Invoke the collection + const result = await Promise.resolve(handler(updatedRequestArgs)); + + // Some handlers may mistakenly return an Error instance instead of throwing. Normalize it to a failure. + if (isErrorLike(result)) { + const err: SerializedError = new Error('Internal error', { cause: { details: result } }); + + err.code = 'INTERNAL_ERROR'; + + throw err; + } + + if (!settled) { + settled = true; + clearTimeout(timer); + ctx.send({ t: 'invoke:result', id: request.id, ok: true, result }); + } + } catch (error) { + if (!settled) { + settled = true; + clearTimeout(timer); + ctx.send({ + t: 'invoke:result', + id: request.id, + ok: false, + error: serializeError(error as Error) + }); + } + } +}; + +/** + * Create the Collections Host: a generic child-process host wired with the record handlers. + * Built-in `hello`/`shutdown` handlers come from `createProcessHost`. + */ +const createCollectionsHost = () => { + let state: HostState = createHostState(); + + return createProcessHost({ + load: async (request, ctx) => { + const loaded = await performLoad(request as LoadRequest); + + state = { + collectionMap: loaded.collectionMap, + descriptors: loaded.descriptors, + invokeTimeoutMs: loaded.invokeTimeoutMs + }; + + ctx.send({ t: 'load:ack', id: request.id, warnings: loaded.warnings, errors: loaded.errors }); + }, + 'manifest:get': (request, ctx) => { + ctx.send({ t: 'manifest:result', id: request.id, collections: state.descriptors }); + }, + invoke: async (request, ctx) => { + await requestInvoke(state, request as InvokeRequest, ctx); + } + }); +}; + +// createProcessHost internally guards on `process.send`, so this is safe at module load. +createCollectionsHost(); + +export { + performLoad, + requestInvoke, + createCollectionsHost +}; diff --git a/src/server.collectionsHostCreator.ts b/src/server.collectionsHostCreator.ts new file mode 100644 index 00000000..50682468 --- /dev/null +++ b/src/server.collectionsHostCreator.ts @@ -0,0 +1,135 @@ +import { type McpCollectionCreator, type McpCollection } from './collections'; + +/** + * Apply a static property to an object. + * + * @private + * @param property - Name of the property to apply + * @param value - Value of the property to apply + * @param obj - Object to apply the property towards + * @returns `true` if the property was applied successfully, `false` otherwise. + */ +const applyStaticProperty = (property: string, value: unknown, obj: unknown) => { + try { + Object.defineProperty(obj, property, { value, writable: false, enumerable: false, configurable: false }); + } catch { + return false; + } + + return true; +}; + +/** + * Guard for an array of creators. File-scoped helper. + * + * @private + * @param value + * @returns `true` if value is an array of functions. + */ +const isCreatorsArray = (value: unknown): value is McpCollectionCreator[] => + Array.isArray(value) && value.length > 0 && value.every(fn => typeof fn === 'function'); + +/** + * Guard for tool tuple. File-scoped helper. + * + * @private + * @param value + * @returns `true` if value is a tool tuple. + */ +const isRealizedTuple = (value: unknown): value is McpCollection => + Array.isArray(value) && + value.length >= 2 && + typeof value[0] === 'string' && + typeof (value as unknown[])[1] === 'function'; + +/** + * Wrap a realized tool tuple in a creator function that returns the tuple itself. + * File-scoped helper. + * + * @private + * @param cached + * @returns A normalized creator function that returns the cached tool tuple. + */ +const wrapCachedTuple = (cached: McpCollection): McpCollectionCreator & { collectionName: string } => { + const wrapped: McpCollectionCreator = () => cached; + + applyStaticProperty('collectionName', cached[0], wrapped); + + return wrapped as McpCollectionCreator & { collectionName: string }; +}; + +/** + * Options for resolveExternalCreators. + */ +type ResolveOptions = { + throwOnEmpty?: boolean; +}; + +/** + * Minimally filter, resolve, then cache tool creators from external module export during the child process. + * + * @param moduleExports - The module exports object from the child process. + * @param options - Options to pass to creators. + * @param settings - Optional settings. + * @param settings.throwOnEmpty - Throw an error if no tool creators are found. Defaults to false. + */ +const resolveCreators = ( + moduleExports: unknown, + options?: Record | undefined, + { throwOnEmpty = false }: ResolveOptions = {} +): McpCollectionCreator[] => { + const mod = moduleExports as any; + const candidates: unknown[] = [mod?.default, mod].filter(Boolean); + + const observed: string[] = []; + + for (const candidate of candidates) { + if (typeof candidate === 'function') { + observed.push('function'); + try { + const result = (candidate as (o?: unknown) => unknown)(options); + + if (isRealizedTuple(result)) { + return [wrapCachedTuple(result)]; + } + + if (isCreatorsArray(result)) { + observed.push('creators[]'); + + return result; + } + + observed.push(Array.isArray(result) ? 'array' : typeof result); + } catch { + // Move to next candidate + } + + continue; + } + + if (isCreatorsArray(candidate)) { + observed.push('creators[]'); + + return candidate as McpCollectionCreator[]; + } + + // Note shape for diagnostics if we end up throwing on empty + observed.push(Array.isArray(candidate) ? 'array' : typeof candidate); + } + + if (throwOnEmpty) { + const shapes = observed.length ? ` Observed candidate shapes: ${observed.join(', ')}` : ''; + + throw new Error([ + `No usable collection creators found from module. ${shapes}`, + 'Expected one of:', + '- default export: a collection creator (function that returns [name, handler])', + '- default export: a function that returns an array of collection creators', + '- default export: an array of collection creators' + ].join('\n')); + } + + return []; +}; + +export { resolveCreators, type ResolveOptions }; diff --git a/src/server.collectionsIpc.ts b/src/server.collectionsIpc.ts new file mode 100644 index 00000000..a42b0efb --- /dev/null +++ b/src/server.collectionsIpc.ts @@ -0,0 +1,90 @@ +import { type CollectionOptions } from './options.collections'; +import { + send, + awaitIpc, + makeId, + matchResponse, + serializeError, + type SerializedError +} from './server.processIpc'; + +/** + * IPC (Inter-Process Communication) request messages. + * + * - `hello` - Sent by the host to the process to acknowledge receipt. + * - `load` - Sent by the host to the process to load tools. + * - `manifest:get` - Sent by the host to the process to request a list of available tools. + * - `invoke` - Sent by the host to the process to invoke a tool. + * - `shutdown` - Sent by the host to the process to shutdown. + * + * @property t - Message type. + * @property id - Message identifier. + * @property specs - List of tool module specifiers to load. + * @property invokeTimeoutMs - Timeout for tool invocations. + * @property {CollectionOptions} options - Options to pass to creators. + * @property session - Session object to pass to creators. + * @property isInternal - Indicates if the request is internal. + */ +type IpcRequest = + | { t: 'hello'; id: string } | + { t: 'load'; id: string; specs: string[]; invokeTimeoutMs?: number; options?: CollectionOptions; session?: unknown; isInternal?: boolean } | + { t: 'manifest:get'; id: string } | + { t: 'invoke'; id: string; collectionId: string; args: unknown; options?: unknown; session?: unknown; isInternal?: boolean } | + { t: 'shutdown'; id: string }; + +/** + * Collection descriptor object for IPC. + * + * @property id - Collection identifier. + * @property name - Collection name. + * @property source - Collection module specifier. + */ +type CollectionDescriptor = { + id: string; + name: string; + source?: string; +}; + +/** + * Inter-Process Communication (IPC) responses. + * + * Types: + * - 'hello:ack': Acknowledgment message for a "hello" operation, including an identifier. + * - 'load:ack': Acknowledgment message for a "load" operation, including an identifier, + * and arrays of warnings and errors. + * - 'manifest:result': Message containing the result of a "manifest" operation, including an + * identifier and a list of tool descriptors. + * - 'invoke:result' (success case): Message containing the result of a successful "invoke" + * operation, including an identifier, a success flag, and the result. + * - 'invoke:result' (failure case): Message containing the result of a failed "invoke" + * operation, including an identifier, a failure flag, and an error descriptor. + * - 'shutdown:ack': Acknowledgment message for a "shutdown" operation, including an identifier. + * + * @property t - Message type. + * @property id - Message identifier. + * @property warnings - List of warnings generated during tool loading. + * @property errors - List of errors generated during tool loading. + * @property {ToolDescriptor[]} tools - List of available tools. + * @property ok - Success flag. + * @property result - Result of the operation. + * @property {SerializedError} error - Error descriptor. + */ +type IpcResponse = + | { t: 'hello:ack'; id: string } | + { t: 'load:ack'; id: string; warnings: string[]; errors: string[] } | + { t: 'manifest:result'; id: string; collections: CollectionDescriptor[] } | + { t: 'invoke:result'; id: string; ok: true; result: unknown } | + { t: 'invoke:result'; id: string; ok: false; error: SerializedError } | + { t: 'shutdown:ack'; id: string }; + +export { + send, + awaitIpc, + makeId, + matchResponse, + serializeError, + type IpcRequest, + type IpcResponse, + type CollectionDescriptor, + type SerializedError +}; diff --git a/src/server.collectionsUser.ts b/src/server.collectionsUser.ts new file mode 100644 index 00000000..ee059cfd --- /dev/null +++ b/src/server.collectionsUser.ts @@ -0,0 +1,222 @@ +import { memo } from './server.caching'; +import { sanitizeDataProp } from './server.toolsUser'; +import { applyStaticProperty } from './server.processUser'; +import { type McpCollection } from './collections'; +import { type GlobalOptions } from './options'; +import { type CollectionOptions } from './options.collections'; + +/** + * Inline tool options. + * + * Alias of {@link GlobalOptions}. + * + * @note Author-facing configuration. + */ +type CollectionInternalOptions = GlobalOptions; + +/** + * External tool options. + * + * Alias of {@link ToolOptions}. + * + * @note Author-facing configuration. + */ +type CollectionExternalOptions = CollectionOptions; + +/** + * A normalized tool entry for normalizing values for strings and tool creators. + * + * @property type - Classification of the entry (tuple, 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 collectionName - The collection name for tuple/object/function entries + * @property error - Error message for invalid entries + */ +type NormalizedCollectionEntry = { + type: 'tuple' | 'invalid'; + index: number; + original: unknown; + value: string | CollectionCreator; + collectionName?: string | undefined; + error?: string | undefined; +}; + +/** + * A general tool entry for normalizing values for creators. + */ +type CreatorEntry = Pick; + +/** + * A function that returns a tuple `Collection`. An MCP collection of records "wrapper", or "creator". + * + * - `CollectionExternalOptions` is a limited subset of `CollectionInternalOptions` for external filePackage creators. + * - `CollectionInternalOptions` is available for inline and built-in collection of records creators. + * + * @note Author-facing configuration. + * @example A creator function. The handler may be async or sync. + * () => [ + * 'creatorRecord', + * async (args) => { ... } + * ] + */ +type CollectionCreator = (options?: CollectionExternalOptions | CollectionInternalOptions) => McpCollection; + +/** + * An array of normalized config values. + * + * - `string` - file path or package id + * - `CollectionCreator` - function creator + * + * @note Author-facing multi-collection configuration. + * @example An array/list of normalized config values + * [ + * './a/file/path/collection.mjs', + * () => [ + * 'creatorCollection', + * async (args) => { ... } + * ] + * ]; + */ +type CollectionModule = ReadonlyArray; + +/** + * Sanitize and return a static collection name. + * + * @param obj + * + * @returns - The sanitized static collection name, or `undefined` if the name is invalid. + */ +const sanitizeStaticCollectionName = (obj: unknown) => { + try { + const name = sanitizeDataProp(obj, 'collectionName'); + + if (typeof name?.value === 'string') { + const trimmed = String.prototype.trim.call(name.value); + + if (trimmed.length > 0) { + return trimmed; + } + } + } catch {} + + return undefined; +}; + +/** + * Normalize a tuple config into a collection of records' creator function. + * + * @param config - The array configuration to normalize. + * @returns A collection of records' creator function, or undefined if the config is invalid. + */ +const normalizeTuple = (config: unknown): CreatorEntry | undefined => { + if (!Array.isArray(config) || config.length < 2) { + return undefined; + } + + const name = sanitizeDataProp(config, '0'); + const handler = sanitizeDataProp(config, '1'); + + if (!name || !handler) { + return undefined; + } + + const updatedName = (name.value as string)?.trim?.() || undefined; + const updatedHandler = typeof handler.value === 'function' ? handler.value : undefined; + + if (!updatedName || !updatedHandler) { + return undefined; + } + + const creator: CollectionCreator = () => [ + updatedName as string, + // updatedHandler as (args: unknown) => unknown | Promise, + updatedHandler, + { + runInChildProcess: true, + isInternal: false + } + ]; + + applyStaticProperty('collectionName', updatedName, creator); + + return { + original: config, + collectionName: updatedName as string, + type: 'tuple', + value: creator + }; +}; + +/** + * Memoize the `normalizeTuple` function. + */ +normalizeTuple.memo = memo(normalizeTuple, { cacheErrors: false, keyHash: args => args[0] }); + +/** + * Normalize the collection of record(s) configuration(s) into a normalized collection entry. + * + * @example Falsy values carried through to retain indexing on messaging + * Input: [ + * () => ['a', { inputSchema: {} }, () => {}], + * undefined, + * { name: 'b', description: 'b', inputSchema: {}, handler: () => {} } + * ] + * Output: ['creator', 'invalid', 'object'] + * + * @param config - The configuration(s) to normalize. + * @returns An array of normalized collection entries. + */ +const normalizeCollections = (config: any): NormalizedCollectionEntry[] => { + const updatedConfigs = (normalizeTuple.memo(config) && [config]) || (Array.isArray(config) && config) || [config]; + const normalizedConfigs: NormalizedCollectionEntry[] = []; + + const flattenedConfigs = updatedConfigs.flatMap((item: unknown) => + (normalizeTuple.memo(item) && [item]) || (Array.isArray(item) && item) || [item]); + + flattenedConfigs.forEach((config: unknown, index: number) => { + if (normalizeTuple.memo(config)) { + normalizedConfigs.push({ + index, + ...normalizeTuple.memo(config) as CreatorEntry + }); + + return; + } + + const err = `createMcpCollection: invalid configuration used at index ${index}: Unsupported type ${typeof config}`; + + normalizedConfigs.push({ + index, + original: config, + type: 'invalid', + value: err, + error: err + }); + }); + + return normalizedConfigs; +}; + +/** + * Memoized version of normalizeCollections. + * + * @note Review the memoization used in server.toolsUser.ts for the final + * implementation. Currently, this is a low-level temporary solution. + */ +normalizeCollections.memo = memo(normalizeCollections, { + cacheErrors: false, + keyHash: args => args[0] +}); + +export { + normalizeCollections, + normalizeTuple, + sanitizeStaticCollectionName, + type CollectionInternalOptions, + type CollectionExternalOptions, + type NormalizedCollectionEntry, + type CollectionCreator, + type CreatorEntry, + type CollectionModule +}; diff --git a/src/server.tools.ts b/src/server.tools.ts index 7c5eb42d..bb041740 100644 --- a/src/server.tools.ts +++ b/src/server.tools.ts @@ -361,11 +361,12 @@ const sendToolsHostShutdown = async ( { pluginHost }: GlobalOptions = getOptions(), { sessionId }: AppSession = getSessionOptions() ): Promise => { - const handle = activeChildrenBySession.get(sessionId) as HostHandle | undefined; + const registryKey = `${sessionId}:tools`; + const handle = activeChildrenBySession.get(registryKey) as HostHandle | undefined; await shutdownChildProcess(handle, { gracePeriodMs: Math.max(0, Number(pluginHost?.gracePeriodMs) || 0), - sessionId, + sessionId: registryKey, label: 'Tools Host' }); }; @@ -388,7 +389,8 @@ const composeTools = async ( { toolModules, nodeVersion, contextUrl, contextPath }: GlobalOptions = getOptions(), { sessionId }: AppSession = getSessionOptions() ): Promise => { - const existingSession = activeChildrenBySession.get(sessionId); + const registryKey = `${sessionId}:tools`; + const existingSession = activeChildrenBySession.get(registryKey); if (existingSession) { log.warn(`Existing Tools Host session detected ${sessionId}. Shutting down the existing host before creating a new one.`); @@ -451,7 +453,7 @@ const composeTools = async ( return; } - const current = activeChildrenBySession.get(sessionId); + const current = activeChildrenBySession.get(registryKey); if (current && current.child === host.child) { try { @@ -461,7 +463,7 @@ const composeTools = async ( log.error(`Failed to close Tools Host stderr reader: ${formatUnknownError(error)}`); } - activeChildrenBySession.delete(sessionId); + activeChildrenBySession.delete(registryKey); } host.child.off('exit', onChildExitOrDisconnect); @@ -492,7 +494,7 @@ const composeTools = async ( const proxiedCreators = makeProxyCreators(filteredHandle); // Associate the spawned host with the current session - activeChildrenBySession.set(sessionId, host); + activeChildrenBySession.set(registryKey, host); host.child.once('exit', onChildExitOrDisconnect); host.child.once('disconnect', onChildExitOrDisconnect); diff --git a/src/server.ts b/src/server.ts index c611821c..8b4fd1a4 100644 --- a/src/server.ts +++ b/src/server.ts @@ -37,7 +37,7 @@ import { type McpCollection, type RegisterCollectionItem } from './collections'; -import { composeCollections } from './server.collections'; +import { composeCollections, sendCollectionsHostShutdown } from './server.collections'; import { setPatternFlyCollection } from './patternFly.getResources'; /** @@ -359,7 +359,10 @@ const runServer = async (options: ServerOptions = getOptions(), { await server?.close(); running = false; - await sendToolsHostShutdown(); + await Promise.allSettled([ + sendToolsHostShutdown(), + sendCollectionsHostShutdown() + ]); log.info(`${options.name} closed!\n`); unsubscribeServerLogger?.(); @@ -545,6 +548,7 @@ runServer.memo = memo( export { runServer, + registerServerCollections, registerServerResources, registerServerTools, type ServerInstance, diff --git a/tests/e2e/__snapshots__/stdioTransport.test.ts.snap b/tests/e2e/__snapshots__/stdioTransport.test.ts.snap index 50256a48..651b7dd7 100644 --- a/tests/e2e/__snapshots__/stdioTransport.test.ts.snap +++ b/tests/e2e/__snapshots__/stdioTransport.test.ts.snap @@ -149,6 +149,8 @@ exports[`Logging should allow setting logging options, stderr 1`] = ` "[INFO]: Server logging enabled. ", "[INFO]: Server stats enabled. +", + "[INFO]: No external collections loaded. ", "[INFO]: No external resources loaded. ",