diff --git a/README.md b/README.md index 75efa1a..ad87067 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,32 @@ Returned content format: - For each entry in urlList, the server loads its content, prefixes it with a header like: `# Documentation from ` and joins multiple entries using a separator: `\n\n---\n\n`. - If an entry fails to load, an inline error message is included for that entry. +## Logging + +The server uses a `diagnostics_channel`–based logger that keeps STDIO stdout pure by default. No terminal output occurs unless you enable a sink. + +- Defaults: `level='info'`, `stderr=false`, `protocol=false` +- Sinks (opt‑in): `--log-stderr`, `--log-protocol` (forwards to MCP clients; requires advertising `capabilities.logging`) +- Transport tag: `transport: 'stdio' | 'http'` (no I/O side effects) +- Environment variables: not used for logging in this version +- Process scope: logger is process‑global; recommend one server per process + +CLI examples: + +```bash +patternfly-mcp # default (no terminal output) +patternfly-mcp --verbose # level=debug (still no stderr) +patternfly-mcp --log-stderr # enable stderr sink +patternfly-mcp --log-level warn --log-stderr +patternfly-mcp --log-protocol --log-level info # forward logs to MCP clients +``` + +Programmatic: + +```ts +await start({ logging: { level: 'info', stderr: false, protocol: false } }); +``` + ### Tool: usePatternFlyDocs Use this to fetch high-level index content (for example, a local README.md that contains relevant links, or llms.txt files in docs-host mode). From that content, you can select specific URLs to pass to fetchDocs. diff --git a/eslint.config.js b/eslint.config.js index f5c0f71..600c872 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -59,7 +59,8 @@ export default [ varsIgnorePattern: '^_' }], 'n/no-process-exit': 0, - 'no-console': 0 + // Disallow console.log/info in runtime to protect STDIO; allow warn/error + 'no-console': ['error', { allow: ['warn', 'error'] }] } }, @@ -73,7 +74,11 @@ export default [ rules: { '@typescript-eslint/no-explicit-any': 0, '@typescript-eslint/ban-ts-comment': 1, - 'no-sparse-arrays': 0 + 'no-sparse-arrays': 0, + // Allow console usage in tests (spies, debug) + 'no-console': 0, + // Relax stylistic padding in tests to reduce churn + '@stylistic/padding-line-between-statements': 0 } } ]; diff --git a/jest.setupTests.ts b/jest.setupTests.ts index d07915e..55f7931 100644 --- a/jest.setupTests.ts +++ b/jest.setupTests.ts @@ -1,5 +1,10 @@ // Shared helpers for Jest unit tests +/** + * Set NODE_ENV to 'local' for local testing. + */ +process.env.NODE_ENV = 'local'; + /** * Note: Mock @patternfly/patternfly-component-schemas/json to avoid top-level await issues in Jest * - Individual tests can override mock diff --git a/package.json b/package.json index 064f95f..eb62058 100644 --- a/package.json +++ b/package.json @@ -23,9 +23,9 @@ "build:clean": "rm -rf dist", "build:watch": "npm run build -- --watch", "release": "changelog --non-cc --link-url https://github.com/patternfly/patternfly-mcp.git", - "start": "node dist/index.js", + "start": "node dist/index.js --verbose --log-stderr", "start:dev": "tsx watch src/index.ts", - "test": "export NODE_ENV=local; npm run test:lint && npm run test:types && jest --roots=src/", + "test": "npm run test:lint && npm run test:types && jest --roots=src/", "test:dev": "npm test -- --watchAll", "test:integration": "npm run build && jest --roots=tests/", "test:integration-dev": "npm run test:integration -- --watchAll", diff --git a/src/__tests__/__snapshots__/index.test.ts.snap b/src/__tests__/__snapshots__/index.test.ts.snap index 24fe62b..39be6b7 100644 --- a/src/__tests__/__snapshots__/index.test.ts.snap +++ b/src/__tests__/__snapshots__/index.test.ts.snap @@ -6,6 +6,11 @@ exports[`main should merge default, cli and programmatic options, merge programm [ { "docsHost": true, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + }, }, ], ], @@ -24,6 +29,11 @@ exports[`main should merge default, cli and programmatic options, merge programm [ { "docsHost": true, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + }, }, ], ], @@ -42,6 +52,11 @@ exports[`main should merge default, cli and programmatic options, with empty pro [ { "docsHost": true, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + }, }, ], ], @@ -60,6 +75,11 @@ exports[`main should merge default, cli and programmatic options, with undefined [ { "docsHost": false, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + }, }, ], ], diff --git a/src/__tests__/__snapshots__/logger.test.ts.snap b/src/__tests__/__snapshots__/logger.test.ts.snap new file mode 100644 index 0000000..19440cd --- /dev/null +++ b/src/__tests__/__snapshots__/logger.test.ts.snap @@ -0,0 +1,197 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`createLogger should activate stderr subscriber writes only at or above level: stderr 1`] = ` +[ + [ + "[INFO]: lorem ipsum :123 {"a":1} +", + ], +] +`; + +exports[`createLogger should attempt to subscribe and unsubscribe from a channel, with no logging options: subscribe 1`] = ` +{ + "subscribe": [], + "unsubscribe": [], +} +`; + +exports[`createLogger should attempt to subscribe and unsubscribe from a channel, with stderr, and emulated channel to pass checks: subscribe 1`] = ` +{ + "subscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], + "unsubscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], +} +`; + +exports[`logSeverity should return log severity, debug 1`] = `0`; + +exports[`logSeverity should return log severity, default 1`] = `-1`; + +exports[`logSeverity should return log severity, error 1`] = `3`; + +exports[`logSeverity should return log severity, info 1`] = `1`; + +exports[`logSeverity should return log severity, warn 1`] = `2`; + +exports[`publish should attempt to create a log entry, args 1`] = ` +{ + "channel": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + ], + ], + "publish": [ + [ + { + "args": [ + "dolor", + "sit", + "amet", + ], + "level": "info", + "msg": "lorem ipsum, info", + "time": 1761955200000, + "transport": "stdio", + }, + ], + ], +} +`; + +exports[`publish should attempt to create a log entry, channel name 1`] = ` +{ + "channel": [ + [ + "custom-channel", + ], + ], + "publish": [ + [ + { + "args": [ + "dolor", + "sit", + "amet", + ], + "level": "info", + "msg": "lorem ipsum, info", + "time": 1761955200000, + "transport": undefined, + }, + ], + ], +} +`; + +exports[`publish should attempt to create a log entry, default 1`] = ` +{ + "channel": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + ], + ], + "publish": [ + [ + { + "level": undefined, + "time": 1761955200000, + "transport": "stdio", + }, + ], + ], +} +`; + +exports[`publish should attempt to create a log entry, level 1`] = ` +{ + "channel": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + ], + ], + "publish": [ + [ + { + "level": "info", + "time": 1761955200000, + "transport": "stdio", + }, + ], + ], +} +`; + +exports[`publish should attempt to create a log entry, msg 1`] = ` +{ + "channel": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + ], + ], + "publish": [ + [ + { + "level": "info", + "msg": "lorem ipsum, info", + "time": 1761955200000, + "transport": "stdio", + }, + ], + ], +} +`; + +exports[`registerStderrSubscriber should activate stderr subscriber writes only at or above level: stderr 1`] = ` +[ + [ + "[INFO]: lorem ipsum :123 {"a":1} +", + ], +] +`; + +exports[`registerStderrSubscriber should attempt to subscribe and unsubscribe from a channel: subscribe 1`] = ` +{ + "subscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], + "unsubscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], +} +`; + +exports[`subscribeToChannel should attempt to subscribe and unsubscribe from a channel: subscribe 1`] = ` +{ + "subscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], + "unsubscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], +} +`; + +exports[`subscribeToChannel should throw an error attempting to subscribe and unsubscribe from a channel: missing channel name 1`] = `"subscribeToChannel called without a configured logging channelName"`; diff --git a/src/__tests__/__snapshots__/options.defaults.test.ts.snap b/src/__tests__/__snapshots__/options.defaults.test.ts.snap index 0c6317e..8fe0234 100644 --- a/src/__tests__/__snapshots__/options.defaults.test.ts.snap +++ b/src/__tests__/__snapshots__/options.defaults.test.ts.snap @@ -2,101 +2,17 @@ exports[`options defaults should return specific properties 1`] = ` { - "DEFAULTS": { - "DEFAULT_OPTIONS": { - "contextPath": "/", - "docsPath": "/documentation", - "llmsFilesPath": "/llms-files", - "name": "@patternfly/patternfly-mcp", - "pfExternal": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content", - "pfExternalAccessibility": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/accessibility", - "pfExternalChartsDesign": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/charts", - "pfExternalDesignComponents": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/components", - "pfExternalDesignLayouts": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/layouts", - "pfExternalExamplesCharts": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-charts/src/victory/components", - "pfExternalExamplesComponents": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-core/src/components", - "pfExternalExamplesLayouts": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-core/src/layouts", - "pfExternalExamplesTable": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-table/src/components", - "repoName": "patternfly-mcp", - "resourceMemoOptions": { - "fetchUrl": { - "cacheErrors": false, - "cacheLimit": 100, - "expire": 180000, - }, - "readFile": { - "cacheErrors": false, - "cacheLimit": 50, - "expire": 120000, - }, - }, - "separator": " - ---- - -", - "toolMemoOptions": { - "fetchDocs": { - "cacheErrors": false, - "cacheLimit": 15, - "expire": 60000, - }, - "usePatternFlyDocs": { - "cacheErrors": false, - "cacheLimit": 10, - "expire": 60000, - }, - }, - "urlRegex": /\\^\\(https\\?:\\)\\\\/\\\\//i, - "version": "0.0.0", - }, - "DEFAULT_SEPARATOR": " - ---- - -", - "PF_EXTERNAL": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content", - "PF_EXTERNAL_ACCESSIBILITY": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/accessibility", - "PF_EXTERNAL_CHARTS_DESIGN": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/charts", - "PF_EXTERNAL_DESIGN_COMPONENTS": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/components", - "PF_EXTERNAL_DESIGN_LAYOUTS": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/layouts", - "PF_EXTERNAL_EXAMPLES": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages", - "PF_EXTERNAL_EXAMPLES_CHARTS": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-charts/src/victory/components", - "PF_EXTERNAL_EXAMPLES_LAYOUTS": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-core/src/layouts", - "PF_EXTERNAL_EXAMPLES_REACT_CORE": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-core/src/components", - "PF_EXTERNAL_EXAMPLES_TABLE": "https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-table/src/components", - "PF_EXTERNAL_EXAMPLES_VERSION": "v6.4.0", - "PF_EXTERNAL_VERSION": "fb05713aba75998b5ecf5299ee3c1a259119bd74", - "RESOURCE_MEMO_OPTIONS": { - "fetchUrl": { - "cacheErrors": false, - "cacheLimit": 100, - "expire": 180000, - }, - "readFile": { - "cacheErrors": false, - "cacheLimit": 50, - "expire": 120000, - }, - }, - "TOOL_MEMO_OPTIONS": { - "fetchDocs": { - "cacheErrors": false, - "cacheLimit": 15, - "expire": 60000, - }, - "usePatternFlyDocs": { - "cacheErrors": false, - "cacheLimit": 10, - "expire": 60000, - }, - }, - "URL_REGEX": /\\^\\(https\\?:\\)\\\\/\\\\//i, - }, "DEFAULT_OPTIONS": { "contextPath": "/", + "docsHost": false, "docsPath": "/documentation", "llmsFilesPath": "/llms-files", + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, "name": "@patternfly/patternfly-mcp", "pfExternal": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content", "pfExternalAccessibility": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/accessibility", @@ -145,6 +61,7 @@ exports[`options defaults should return specific properties 1`] = ` --- ", + "LOG_BASENAME": "pf-mcp:log", "PF_EXTERNAL": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content", "PF_EXTERNAL_ACCESSIBILITY": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/accessibility", "PF_EXTERNAL_CHARTS_DESIGN": "https://raw.githubusercontent.com/patternfly/patternfly-org/fb05713aba75998b5ecf5299ee3c1a259119bd74/packages/documentation-site/patternfly-docs/content/design-guidelines/charts", diff --git a/src/__tests__/__snapshots__/options.test.ts.snap b/src/__tests__/__snapshots__/options.test.ts.snap index c0d8cde..f62303d 100644 --- a/src/__tests__/__snapshots__/options.test.ts.snap +++ b/src/__tests__/__snapshots__/options.test.ts.snap @@ -3,17 +3,83 @@ exports[`parseCliOptions should attempt to parse args with --docs-host flag 1`] = ` { "docsHost": true, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, +} +`; + +exports[`parseCliOptions should attempt to parse args with --log-level flag 1`] = ` +{ + "docsHost": false, + "logging": { + "level": "warn", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, +} +`; + +exports[`parseCliOptions should attempt to parse args with --log-stderr flag and --log-protocol flag 1`] = ` +{ + "docsHost": false, + "logging": { + "level": "info", + "protocol": true, + "stderr": true, + "transport": "stdio", + }, +} +`; + +exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] = ` +{ + "docsHost": false, + "logging": { + "level": "debug", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, +} +`; + +exports[`parseCliOptions should attempt to parse args with --verbose flag and --log-level flag 1`] = ` +{ + "docsHost": false, + "logging": { + "level": "debug", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, } `; exports[`parseCliOptions should attempt to parse args with other arguments 1`] = ` { "docsHost": false, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, } `; exports[`parseCliOptions should attempt to parse args without --docs-host flag 1`] = ` { "docsHost": false, + "logging": { + "level": "info", + "protocol": false, + "stderr": false, + "transport": "stdio", + }, } `; diff --git a/src/__tests__/__snapshots__/server.helpers.test.ts.snap b/src/__tests__/__snapshots__/server.helpers.test.ts.snap index 7675972..64571f7 100644 --- a/src/__tests__/__snapshots__/server.helpers.test.ts.snap +++ b/src/__tests__/__snapshots__/server.helpers.test.ts.snap @@ -69,3 +69,40 @@ exports[`generateHash should generate a consistent hash, symbols, different inst exports[`generateHash should generate a consistent hash, symbols, different instances BUT with same string will return the same 1`] = `"b3f594684e6b3954940c8f206a54a79f153a2424"`; exports[`generateHash should generate a consistent hash, undefined 1`] = `"19dafbceef2fcb380547ee9dc4c690a2f9e08d07"`; + +exports[`mergeObjects should merge two objects, non-objects 1`] = `"lorem"`; + +exports[`mergeObjects should merge two objects, plain object against array 1`] = `{}`; + +exports[`mergeObjects should merge two objects, plain object against null 1`] = `{}`; + +exports[`mergeObjects should merge two objects, plain object against plain object 1`] = `{}`; + +exports[`mergeObjects should merge two objects, plain object against undefined 1`] = `{}`; + +exports[`mergeObjects should merge two objects, recursive plain object against recursive plain object 1`] = ` +{ + "lorem": { + "adipiscing": null, + "amet": { + "consectetur": "adipiscing", + "elit": { + "dolor": "magna", + "sed": "do", + }, + }, + "consectetur": [Function], + "dolor": [ + "sit", + "amet", + ], + "elit": { + "sed": "do", + }, + "ipsum": "dolor", + "sit": [ + "amet", + ], + }, +} +`; diff --git a/src/__tests__/__snapshots__/server.logger.test.ts.snap b/src/__tests__/__snapshots__/server.logger.test.ts.snap new file mode 100644 index 0000000..c672c59 --- /dev/null +++ b/src/__tests__/__snapshots__/server.logger.test.ts.snap @@ -0,0 +1,92 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`createServerLogger should attempt to subscribe and unsubscribe from a channel, with no logging options: subscribe 1`] = ` +{ + "subscribe": [], + "unsubscribe": [], +} +`; + +exports[`createServerLogger should attempt to subscribe and unsubscribe from a channel, with stderr, and emulated channel to pass checks: subscribe 1`] = ` +{ + "subscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], + "unsubscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], +} +`; + +exports[`createServerLogger should attempt to subscribe and unsubscribe from a channel, with stderr, protocol, and emulated channel to pass checks: subscribe 1`] = ` +{ + "subscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], + "unsubscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], +} +`; + +exports[`createServerLogger should return a memoized server logger that avoids duplicate sinks; teardown stops emissions: stderr 1`] = ` +[ + [ + "[INFO]: lorem ipsum, dolor sit info +", + ], + [ + "[INFO]: dolor sit amet +", + ], +] +`; + +exports[`registerMcpSubscriber should attempt to subscribe and unsubscribe from a channel: subscribe 1`] = ` +{ + "subscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], + "unsubscribe": [ + [ + "pf-mcp:log:1234d567-1ce9-123d-1413-a1234e56c789", + [Function], + ], + ], +} +`; + +exports[`toMcpLevel should return log severity, anything 1`] = `"error"`; + +exports[`toMcpLevel should return log severity, debug 1`] = `"debug"`; + +exports[`toMcpLevel should return log severity, default 1`] = `"error"`; + +exports[`toMcpLevel should return log severity, error 1`] = `"error"`; + +exports[`toMcpLevel should return log severity, info 1`] = `"info"`; + +exports[`toMcpLevel should return log severity, warn 1`] = `"warning"`; diff --git a/src/__tests__/__snapshots__/server.test.ts.snap b/src/__tests__/__snapshots__/server.test.ts.snap index 7a7528b..afd158a 100644 --- a/src/__tests__/__snapshots__/server.test.ts.snap +++ b/src/__tests__/__snapshots__/server.test.ts.snap @@ -1,9 +1,8 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing -exports[`runServer should attempt to run server, create transport, connect, and log success message: console 1`] = ` +exports[`runServer should attempt to run server, create transport, connect, and log success message: diagnostics 1`] = ` { - "info": [], - "log": [ + "events": [ [ "PatternFly MCP server running on stdio", ], @@ -31,10 +30,9 @@ exports[`runServer should attempt to run server, create transport, connect, and } `; -exports[`runServer should attempt to run server, disable SIGINT handler: console 1`] = ` +exports[`runServer should attempt to run server, disable SIGINT handler: diagnostics 1`] = ` { - "info": [], - "log": [ + "events": [ [ "PatternFly MCP server running on stdio", ], @@ -57,10 +55,9 @@ exports[`runServer should attempt to run server, disable SIGINT handler: console } `; -exports[`runServer should attempt to run server, enable SIGINT handler explicitly: console 1`] = ` +exports[`runServer should attempt to run server, enable SIGINT handler explicitly: diagnostics 1`] = ` { - "info": [], - "log": [ + "events": [ [ "PatternFly MCP server running on stdio", ], @@ -88,14 +85,12 @@ exports[`runServer should attempt to run server, enable SIGINT handler explicitl } `; -exports[`runServer should attempt to run server, register a tool: console 1`] = ` +exports[`runServer should attempt to run server, register a tool: diagnostics 1`] = ` { - "info": [ + "events": [ [ "Registered tool: loremIpsum", ], - ], - "log": [ [ "PatternFly MCP server running on stdio", ], @@ -132,17 +127,15 @@ exports[`runServer should attempt to run server, register a tool: console 1`] = } `; -exports[`runServer should attempt to run server, register multiple tools: console 1`] = ` +exports[`runServer should attempt to run server, register multiple tools: diagnostics 1`] = ` { - "info": [ + "events": [ [ "Registered tool: loremIpsum", ], [ "Registered tool: dolorSit", ], - ], - "log": [ [ "PatternFly MCP server running on stdio", ], @@ -187,10 +180,9 @@ exports[`runServer should attempt to run server, register multiple tools: consol } `; -exports[`runServer should attempt to run server, use custom options: console 1`] = ` +exports[`runServer should attempt to run server, use custom options: diagnostics 1`] = ` { - "info": [], - "log": [ + "events": [ [ "PatternFly MCP server running on stdio", ], @@ -218,9 +210,9 @@ exports[`runServer should attempt to run server, use custom options: console 1`] } `; -exports[`runServer should attempt to run server, use default tools: console 1`] = ` +exports[`runServer should attempt to run server, use default tools: diagnostics 1`] = ` { - "info": [ + "events": [ [ "Registered tool: usePatternFlyDocs", ], @@ -230,8 +222,6 @@ exports[`runServer should attempt to run server, use default tools: console 1`] [ "Registered tool: componentSchemas", ], - ], - "log": [ [ "PatternFly MCP server running on stdio", ], @@ -730,3 +720,7 @@ exports[`runServer should attempt to run server, use default tools: console 1`] ], } `; + +exports[`runServer should handle errors during connection: Connection failed 1`] = `"Connection failed"`; + +exports[`runServer should handle errors during server creation: Server creation failed 1`] = `"Server creation failed"`; diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 39ad80f..af15156 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -17,6 +17,7 @@ describe('main', () => { let consoleErrorSpy: jest.SpyInstance; let processExitSpy: jest.SpyInstance; let callOrder: string[] = []; + const defaultLogging = { level: 'info' as const, stderr: false, protocol: false }; beforeEach(() => { jest.clearAllMocks(); @@ -32,7 +33,7 @@ describe('main', () => { mockParseCliOptions.mockImplementation(() => { callOrder.push('parse'); - return { docsHost: false }; + return { docsHost: false, logging: defaultLogging } as unknown as CliOptions; }); mockSetOptions.mockImplementation(options => { callOrder.push('set'); @@ -130,10 +131,10 @@ describe('main', () => { mockParseCliOptions.mockImplementation(() => { callOrder.push('parse'); - return cliOptions; + return { ...(cliOptions as any), logging: defaultLogging } as unknown as CliOptions; }); - await method(programmaticOptions); + await method(programmaticOptions as any); expect({ methodRegistersAs: method.name, diff --git a/src/__tests__/logger.test.ts b/src/__tests__/logger.test.ts new file mode 100644 index 0000000..d5f17d8 --- /dev/null +++ b/src/__tests__/logger.test.ts @@ -0,0 +1,221 @@ +import diagnostics_channel from 'node:diagnostics_channel'; +import { setOptions, getLoggerOptions } from '../options.context'; +import { logSeverity, publish, subscribeToChannel, registerStderrSubscriber, createLogger } from '../logger'; + +describe('logSeverity', () => { + it.each([ + { + description: 'default', + param: undefined + }, + { + description: 'debug', + param: 'debug' + }, + { + description: 'info', + param: 'info' + }, + { + description: 'warn', + param: 'warn' + }, + { + description: 'error', + param: 'error' + } + ])('should return log severity, $description', ({ param }) => { + expect(logSeverity(param as any)).toMatchSnapshot(); + }); +}); + +describe('publish', () => { + let channelSpy: jest.SpyInstance; + const mockPublish = jest.fn(); + + beforeEach(() => { + jest.useFakeTimers(); // Use modern fake timers + jest.setSystemTime(new Date('2025-11-01T00:00:00Z')); + channelSpy = jest.spyOn(diagnostics_channel, 'channel'); + channelSpy.mockImplementation(() => ({ publish: mockPublish })); + }); + + afterEach(() => { + mockPublish.mockClear(); + channelSpy.mockRestore(); + jest.useRealTimers(); + }); + + it.each([ + { + description: 'default', + level: undefined, + options: undefined, + msg: undefined, + args: [] + }, + { + description: 'level', + level: 'info', + options: undefined, + msg: undefined, + args: [] + }, + { + description: 'msg', + level: 'info', + options: undefined, + msg: 'lorem ipsum, info', + args: [] + }, + { + description: 'args', + level: 'info', + options: undefined, + msg: 'lorem ipsum, info', + args: ['dolor', 'sit', 'amet'] + }, + { + description: 'channel name', + level: 'info', + options: { channelName: 'custom-channel' }, + msg: 'lorem ipsum, info', + args: ['dolor', 'sit', 'amet'] + } + ])('should attempt to create a log entry, $description', ({ level, options, msg, args }) => { + publish(level as any, options as any, msg, ...args); + + expect({ + channel: channelSpy.mock.calls, + publish: mockPublish.mock.calls + }).toMatchSnapshot(); + }); +}); + +describe('subscribeToChannel', () => { + let subscribeSpy: jest.SpyInstance; + let unsubscribeSpy: jest.SpyInstance; + + beforeEach(() => { + subscribeSpy = jest.spyOn(diagnostics_channel, 'subscribe'); + unsubscribeSpy = jest.spyOn(diagnostics_channel, 'unsubscribe'); + }); + + afterEach(() => { + subscribeSpy.mockRestore(); + unsubscribeSpy.mockRestore(); + }); + + it('should attempt to subscribe and unsubscribe from a channel', () => { + const handler = jest.fn(); + const unsubscribe = subscribeToChannel(handler); + + unsubscribe(); + + expect({ + subscribe: subscribeSpy.mock.calls, + unsubscribe: unsubscribeSpy.mock.calls + }).toMatchSnapshot('subscribe'); + }); + + it('should throw an error attempting to subscribe and unsubscribe from a channel', () => { + expect(() => subscribeToChannel(jest.fn(), { channelName: undefined } as any)) + .toThrowErrorMatchingSnapshot('missing channel name'); + }); +}); + +describe('registerStderrSubscriber', () => { + let subscribeSpy: jest.SpyInstance; + let unsubscribeSpy: jest.SpyInstance; + + beforeEach(() => { + setOptions({}); + subscribeSpy = jest.spyOn(diagnostics_channel, 'subscribe'); + unsubscribeSpy = jest.spyOn(diagnostics_channel, 'unsubscribe'); + }); + + afterEach(() => { + subscribeSpy.mockRestore(); + unsubscribeSpy.mockRestore(); + }); + + it('should attempt to subscribe and unsubscribe from a channel', () => { + const unsubscribe = registerStderrSubscriber({ channelName: 'loremIpsum', level: 'info' } as any); + + unsubscribe(); + + expect({ + subscribe: subscribeSpy.mock.calls, + unsubscribe: unsubscribeSpy.mock.calls + }).toMatchSnapshot('subscribe'); + }); + + it('should activate stderr subscriber writes only at or above level', () => { + setOptions({ logging: { stderr: true, level: 'info' } as any }); + + const stderrSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true as any); + + const unsubscribe = registerStderrSubscriber(getLoggerOptions()); + publish('debug', getLoggerOptions(), 'debug suppressed'); + publish('info', getLoggerOptions(), 'lorem ipsum', 123, { a: 1 }); + + expect(stderrSpy.mock.calls).toMatchSnapshot('stderr'); + + unsubscribe(); + stderrSpy.mockRestore(); + }); +}); + +describe('createLogger', () => { + let subscribeSpy: jest.SpyInstance; + let unsubscribeSpy: jest.SpyInstance; + + beforeEach(() => { + setOptions({}); + subscribeSpy = jest.spyOn(diagnostics_channel, 'subscribe'); + unsubscribeSpy = jest.spyOn(diagnostics_channel, 'unsubscribe'); + }); + + afterEach(() => { + subscribeSpy.mockRestore(); + unsubscribeSpy.mockRestore(); + }); + + it.each([ + { + description: 'with stderr, and emulated channel to pass checks', + channelName: 'loremIpsum', + stderr: true + }, + { + description: 'with no logging options', + channelName: undefined, + stderr: false + } + ])('should attempt to subscribe and unsubscribe from a channel, $description', ({ channelName, stderr }) => { + // Use channelName to pass conditions + const unsubscribe = createLogger({ channelName, stderr } as any); + + unsubscribe(); + + expect({ + subscribe: subscribeSpy.mock.calls, + unsubscribe: unsubscribeSpy.mock.calls + }).toMatchSnapshot('subscribe'); + }); + + it('should activate stderr subscriber writes only at or above level', () => { + setOptions({ logging: { stderr: true, level: 'info' } as any }); + + const stderrSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true as any); + + const unsubscribe = createLogger(getLoggerOptions()); + publish('debug', getLoggerOptions(), 'debug suppressed'); + publish('info', getLoggerOptions(), 'lorem ipsum', 123, { a: 1 }); + + expect(stderrSpy.mock.calls).toMatchSnapshot('stderr'); + + unsubscribe(); + stderrSpy.mockRestore(); + }); +}); diff --git a/src/__tests__/options.context.test.ts b/src/__tests__/options.context.test.ts index 38f6401..f970f01 100644 --- a/src/__tests__/options.context.test.ts +++ b/src/__tests__/options.context.test.ts @@ -2,6 +2,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { runServer, type McpTool } from '../server'; import { getOptions, setOptions } from '../options.context'; +import { DEFAULT_OPTIONS } from '../options.defaults'; // Mock dependencies jest.mock('@modelcontextprotocol/sdk/server/mcp.js'); @@ -10,6 +11,29 @@ jest.mock('@modelcontextprotocol/sdk/server/stdio.js'); const MockMcpServer = McpServer as jest.MockedClass; const MockStdioServerTransport = StdioServerTransport as jest.MockedClass; +describe('setOptions', () => { + it('should ignore valid but incorrect options for merged options', () => { + const updatedOptions = setOptions({ logging: 'oops' as any, resourceMemoOptions: 'gotcha' as any, toolMemoOptions: 'really?' as any }); + + expect(updatedOptions.logging.protocol).toBe(DEFAULT_OPTIONS.logging.protocol); + expect(updatedOptions.resourceMemoOptions?.readFile?.expire).toBe(DEFAULT_OPTIONS.resourceMemoOptions?.readFile?.expire); + expect(updatedOptions.toolMemoOptions?.fetchDocs?.expire).toBe(DEFAULT_OPTIONS.toolMemoOptions?.fetchDocs?.expire); + }); + + it('should ignore null/invalid nested overrides safely', () => { + const updatedOptions = setOptions({ logging: null as any, resourceMemoOptions: null as any }); + + expect(typeof updatedOptions.logging.protocol === 'boolean').toBe(true); + expect(updatedOptions.logging.protocol).toBe(DEFAULT_OPTIONS.logging.protocol); + + expect(typeof updatedOptions.resourceMemoOptions?.readFile?.expire === 'number').toBe(true); + expect(updatedOptions.resourceMemoOptions?.readFile?.expire).toBe(DEFAULT_OPTIONS.resourceMemoOptions?.readFile?.expire); + + expect(typeof updatedOptions.toolMemoOptions?.fetchDocs?.expire === 'number').toBe(true); + expect(updatedOptions.toolMemoOptions?.fetchDocs?.expire).toBe(DEFAULT_OPTIONS.toolMemoOptions?.fetchDocs?.expire); + }); +}); + describe('apply context options', () => { it.each([ { @@ -46,6 +70,7 @@ describe('tool creator options context', () => { let mockTransport: any; beforeEach(() => { + setOptions({}); jest.clearAllMocks(); // Mock server instance diff --git a/src/__tests__/options.test.ts b/src/__tests__/options.test.ts index 5ff80a8..de3786e 100644 --- a/src/__tests__/options.test.ts +++ b/src/__tests__/options.test.ts @@ -16,6 +16,22 @@ describe('parseCliOptions', () => { description: 'without --docs-host flag', args: ['node', 'script.js'] }, + { + description: 'with --verbose flag', + args: ['node', 'script.js', '--verbose'] + }, + { + description: 'with --verbose flag and --log-level flag', + args: ['node', 'script.js', '--verbose', '--log-level', 'warn'] + }, + { + description: 'with --log-level flag', + args: ['node', 'script.js', '--log-level', 'warn'] + }, + { + description: 'with --log-stderr flag and --log-protocol flag', + args: ['node', 'script.js', '--log-stderr', '--log-protocol'] + }, { description: 'with other arguments', args: ['node', 'script.js', 'other', 'args'] diff --git a/src/__tests__/server.helpers.test.ts b/src/__tests__/server.helpers.test.ts index a1b631b..e15c250 100644 --- a/src/__tests__/server.helpers.test.ts +++ b/src/__tests__/server.helpers.test.ts @@ -1,4 +1,67 @@ -import { generateHash, hashCode, isPlainObject, isPromise } from '../server.helpers'; +import { freezeObject, generateHash, hashCode, isPlainObject, isPromise, mergeObjects } from '../server.helpers'; + +describe('freezeObject', () => { + it.each([ + { + description: 'null', + obj: null + }, + { + description: 'undefined', + obj: undefined + }, + { + description: 'plain object', + obj: { lorem: 'ipsum' } + }, + { + description: 'array', + obj: [1, 2, 3] + }, + { + description: 'number', + obj: 1 + }, + { + description: 'string', + obj: 'lorem ipsum' + }, + { + description: 'boolean', + obj: true + }, + { + description: 'function', + obj: () => 'lorem ipsum' + }, + { + description: 'symbol', + obj: Symbol('lorem ipsum') + }, + { + description: 'error', + obj: new Error('lorem ipsum') + }, + { + description: 'date', + obj: new Date('2023-01-01') + }, + { + description: 'regex', + obj: /lorem/g + }, + { + description: 'map', + obj: new Map([['lorem', 1], ['ipsum', 2]]) + }, + { + description: 'set', + obj: new Set([1, 2, 3]) + } + ])('should freeze an object, $description', ({ obj, outcome = true }: any = {}) => { + expect(Object.isFrozen(freezeObject(obj))).toBe(outcome); + }); +}); describe('generateHash', () => { it.each([ @@ -366,3 +429,85 @@ describe('isPlainObject', () => { expect(isPlainObject(param)).toBe(value); }); }); + +describe('mergeObjects', () => { + it.each([ + { + description: 'non-objects', + obj1: 'lorem', + obj2: 'ipsum' + }, + { + description: 'plain object against plain object', + obj1: {}, + obj2: {} + }, + { + description: 'plain object against undefined', + obj1: {}, + obj2: undefined + }, + { + description: 'plain object against null', + obj1: {}, + obj2: null + }, + { + description: 'plain object against array', + obj1: {}, + obj2: [] + }, + { + description: 'recursive plain object against recursive plain object', + obj1: { + lorem: { + ipsum: 'lorem', + sit: ['ipsum'], + dolor: [ + { + amet: 'consectetur' + } + ], + amet: { + consectetur: 'adipiscing', + elit: { + sed: 'do' + } + }, + adipiscing: { + consectetur: 'elit' + }, + elit: { + sed: 'do' + } + } + }, + obj2: { + lorem: { + ipsum: 'dolor', + sit: ['amet'], + dolor: ['sit', 'amet'], + consectetur: () => 'adipiscing', + amet: { + elit: { + dolor: 'magna' + } + }, + adipiscing: null, + elit: { + sed: undefined + } + } + } + } + ])('should merge two objects, $description', ({ obj1, obj2 }) => { + expect(mergeObjects(obj1 as any, obj2 as any)).toMatchSnapshot(); + }); + + it('mergeObjects should ignore prototype pollution keys', () => { + const merged = mergeObjects({}, { __proto__: { polluted: true } }); + + expect((merged as any).polluted).toBeUndefined(); + expect((Object.prototype as any).polluted).toBeUndefined(); + }); +}); diff --git a/src/__tests__/server.logger.test.ts b/src/__tests__/server.logger.test.ts new file mode 100644 index 0000000..a829407 --- /dev/null +++ b/src/__tests__/server.logger.test.ts @@ -0,0 +1,130 @@ +import diagnostics_channel from 'node:diagnostics_channel'; +import { getOptions, setOptions } from '../options.context'; +import { toMcpLevel, registerMcpSubscriber, createServerLogger } from '../server.logger'; +import { log } from '../logger'; + +describe('toMcpLevel', () => { + it.each([ + { + description: 'default', + level: undefined + }, + { + description: 'debug', + level: 'debug' + }, + { + description: 'info', + level: 'info' + }, + { + description: 'warn', + level: 'warn' + }, + { + description: 'error', + level: 'error' + }, + { + description: 'anything', + level: 'lorem ipsum' + } + ])('should return log severity, $description', ({ level }) => { + expect(toMcpLevel(level as any)).toMatchSnapshot(); + }); +}); + +describe('registerMcpSubscriber', () => { + let subscribeSpy: jest.SpyInstance; + let unsubscribeSpy: jest.SpyInstance; + + beforeEach(() => { + setOptions({}); + subscribeSpy = jest.spyOn(diagnostics_channel, 'subscribe'); + unsubscribeSpy = jest.spyOn(diagnostics_channel, 'unsubscribe'); + }); + + afterEach(() => { + subscribeSpy.mockRestore(); + unsubscribeSpy.mockRestore(); + }); + + it('should attempt to subscribe and unsubscribe from a channel', () => { + const options = getOptions(); + const unsubscribe = registerMcpSubscriber((() => {}) as any, options); + + unsubscribe(); + + expect({ + subscribe: subscribeSpy.mock.calls, + unsubscribe: unsubscribeSpy.mock.calls + }).toMatchSnapshot('subscribe'); + }); +}); + +describe('createServerLogger', () => { + let subscribeSpy: jest.SpyInstance; + let unsubscribeSpy: jest.SpyInstance; + + beforeEach(() => { + setOptions({}); + subscribeSpy = jest.spyOn(diagnostics_channel, 'subscribe'); + unsubscribeSpy = jest.spyOn(diagnostics_channel, 'unsubscribe'); + }); + + afterEach(() => { + subscribeSpy.mockRestore(); + unsubscribeSpy.mockRestore(); + }); + + it.each([ + { + description: 'with stderr, and emulated channel to pass checks', + options: { logging: { channelName: 'loremIpsum', stderr: true, protocol: false } } + }, + { + description: 'with stderr, protocol, and emulated channel to pass checks', + options: { logging: { channelName: 'loremIpsum', stderr: true, protocol: true } } + }, + { + description: 'with no logging options', + options: { logging: {} }, + stderr: false + } + ])('should attempt to subscribe and unsubscribe from a channel, $description', ({ options }) => { + // Use channelName to pass conditions + const unsubscribe = createServerLogger((() => {}) as any, options as any); + + unsubscribe(); + + expect({ + subscribe: subscribeSpy.mock.calls, + unsubscribe: unsubscribeSpy.mock.calls + }).toMatchSnapshot('subscribe'); + }); + + it('should return a memoized server logger that avoids duplicate sinks; teardown stops emissions', () => { + setOptions({ logging: { stderr: true, level: 'info' } as any }); + + const stderrSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true as any); + class MockServer { sendLoggingMessage = jest.fn(async () => {}); } + const server = new MockServer() as any; + + const unsubscribeCallOne = createServerLogger.memo(server); + const unsubscribeCallTwo = createServerLogger.memo(server); + + log.info('lorem ipsum, dolor sit info'); + + expect(unsubscribeCallOne).toBe(unsubscribeCallTwo); + + log.info('dolor sit amet'); + + unsubscribeCallOne(); + + log.info('hello world!'); + + expect(stderrSpy.mock.calls).toMatchSnapshot('stderr'); + + stderrSpy.mockRestore(); + }); +}); diff --git a/src/__tests__/server.test.ts b/src/__tests__/server.test.ts index 11c0196..a370b97 100644 --- a/src/__tests__/server.test.ts +++ b/src/__tests__/server.test.ts @@ -2,20 +2,25 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { runServer } from '../server'; import { type GlobalOptions } from '../options'; +import { log } from '../logger'; // Mock dependencies jest.mock('@modelcontextprotocol/sdk/server/mcp.js'); jest.mock('@modelcontextprotocol/sdk/server/stdio.js'); +jest.mock('../logger'); +jest.mock('../server.logger', () => ({ + createServerLogger: { + memo: jest.fn().mockImplementation(() => {}) + } +})); const MockMcpServer = McpServer as jest.MockedClass; const MockStdioServerTransport = StdioServerTransport as jest.MockedClass; +const MockLog = log as jest.MockedObject; describe('runServer', () => { let mockServer: any; let mockTransport: any; - let consoleInfoSpy: jest.SpyInstance; - let consoleLogSpy: jest.SpyInstance; - let consoleErrorSpy: jest.SpyInstance; let processOnSpy: jest.SpyInstance; beforeEach(() => { @@ -34,19 +39,11 @@ describe('runServer', () => { MockMcpServer.mockImplementation(() => mockServer); MockStdioServerTransport.mockImplementation(() => mockTransport); - // Spy on console methods - consoleInfoSpy = jest.spyOn(console, 'info').mockImplementation(); - consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(); - consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); - // Spy on process.on method processOnSpy = jest.spyOn(process, 'on').mockImplementation(); }); afterEach(() => { - consoleInfoSpy.mockRestore(); - consoleLogSpy.mockRestore(); - consoleErrorSpy.mockRestore(); processOnSpy.mockRestore(); }); @@ -61,6 +58,7 @@ describe('runServer', () => { options: { name: 'test-server', version: '1.0.0' + // logging: { protocol: false } }, tools: [] }, @@ -117,13 +115,13 @@ describe('runServer', () => { await runServer(options as GlobalOptions, Object.keys(settings).length > 0 ? settings : undefined); expect(MockStdioServerTransport).toHaveBeenCalled(); + expect({ - info: consoleInfoSpy.mock.calls, + events: MockLog.info.mock.calls, registerTool: mockServer.registerTool.mock.calls, mcpServer: MockMcpServer.mock.calls, - log: consoleLogSpy.mock.calls, process: processOnSpy.mock.calls - }).toMatchSnapshot('console'); + }).toMatchSnapshot('diagnostics'); }); it('should handle errors during server creation', async () => { @@ -133,8 +131,7 @@ describe('runServer', () => { throw error; }); - await expect(runServer(undefined, { tools: [] })).rejects.toThrow('Server creation failed'); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error creating MCP server:', error); + await expect(runServer(undefined, { tools: [] })).rejects.toThrowErrorMatchingSnapshot('Server creation failed'); }); it('should handle errors during connection', async () => { @@ -142,7 +139,6 @@ describe('runServer', () => { mockServer.connect.mockRejectedValue(error); - await expect(runServer(undefined, { tools: [] })).rejects.toThrow('Connection failed'); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error creating MCP server:', error); + await expect(runServer(undefined, { tools: [] })).rejects.toThrowErrorMatchingSnapshot('Connection failed'); }); }); diff --git a/src/index.ts b/src/index.ts index ab202a3..e7b2d8c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,6 @@ #!/usr/bin/env node -import { parseCliOptions, type CliOptions } from './options'; +import { parseCliOptions, type CliOptions, type DefaultOptions } from './options'; import { setOptions } from './options.context'; import { runServer, type ServerInstance } from './server'; @@ -10,16 +10,17 @@ import { runServer, type ServerInstance } from './server'; * @param programmaticOptions - Optional programmatic options that override CLI options * @returns {Promise} Server-instance with shutdown capability */ -const main = async (programmaticOptions?: Partial): Promise => { +const main = async (programmaticOptions?: Partial): Promise => { try { // Parse CLI options const cliOptions = parseCliOptions(); - // Apply options to context. setOptions merges with DEFAULT_OPTIONS internally + // Apply options to context. setOptions merges with session and DEFAULT_OPTIONS internally setOptions({ ...cliOptions, ...programmaticOptions }); return await runServer(); } catch (error) { + // Use console.error, log.error requires initialization console.error('Failed to start server:', error); process.exit(1); } @@ -28,6 +29,7 @@ const main = async (programmaticOptions?: Partial): Promise { + // Use console.error, log.error requires initialization console.error('Failed to start server:', error); process.exit(1); }); diff --git a/src/logger.ts b/src/logger.ts new file mode 100644 index 0000000..66cee7e --- /dev/null +++ b/src/logger.ts @@ -0,0 +1,197 @@ +import { channel, unsubscribe, subscribe } from 'node:diagnostics_channel'; +import { type LoggingSession } from './options.defaults'; +import { getLoggerOptions } from './options.context'; + +type LogLevel = LoggingSession['level']; + +/** + * Log an event with detailed information about a specific action. + * + * @interface LogEvent + * @property {LogLevel} level - Severity level of the event. + * @property [msg] - Optional Message providing context or description of the event. + * @property [args] - Optional additional arguments associated with the event. + * @property [fields] - Optional key-value pairs for metadata associated with the event. + * @property time - Event timestamp in epoch milliseconds. + * @property [source] - Name of the module or subsystem generating the event, if available. + * @property {LoggingSession['transport']} [transport] - Transport configuration used for this event. + */ +interface LogEvent { + level: LogLevel; + msg?: string; + args?: unknown[]; + fields?: Record; + time: number; // epoch ms + source?: string; // optional module/subsystem name + transport?: LoggingSession['transport']; +} + +/** + * Log level ordering used for filtering. Levels are ordered specifically from the least to the most severe. + */ +const LOG_LEVELS: LogLevel[] = ['debug', 'info', 'warn', 'error']; + +/** + * Convert a severity level to a numeric representation from `LogLevel[]`. + * + * @param level - The log level to evaluate. + * @returns Numeric index corresponding to `LogLevel[]` Returns + * -1 if the level is not found. + */ +const logSeverity = (level: unknown): number => + LOG_LEVELS.indexOf(level as LogLevel); + +/** + * Publish a structured log event to the diagnostics channel. + * + * @param level - Log level for the event + * @param {LoggingSession} [options] + * @param [msg] - Optional log message (string) or first argument + * @param [args] - Optional additional arguments for the log event + */ +const publish = (level: LogLevel, options: LoggingSession = getLoggerOptions(), msg?: unknown, ...args: unknown[]) => { + const channelName = options?.channelName; + const timestamp = Date.now(); + const event: LogEvent = { level, time: timestamp }; + + // If first arg is a string, treat it as the message and capture rest as args + if (typeof msg === 'string') { + event.msg = msg; + + if (args.length) { + event.args = args; + } + } else { + const arr = [msg, ...args].filter(v => v !== undefined); + + if (arr.length) { + event.args = arr as unknown[]; + } + } + + event.transport = options?.transport; + + if (channelName) { + channel(channelName).publish(event); + } +}; + +/** + * Subscribe to the diagnostics channel and invoke a handler for each event. + * + * If the event doesn't contain a valid `level` property, the handler is not invoked. + * + * @param handler - Callback function to handle log events + * @param {LoggingSession} [options] + * @returns Function to unsubscribe from the log channel + */ +const subscribeToChannel = (handler: (message: LogEvent) => void, options: LoggingSession = getLoggerOptions()) => { + const channelName = options?.channelName; + + if (!channelName) { + throw new Error('subscribeToChannel called without a configured logging channelName'); + } + + const updatedHandler = (event: LogEvent) => { + if (!event?.level) { + return; + } + + handler.call(null, event); + }; + + subscribe(channelName, updatedHandler as (message: unknown) => void); + + return () => unsubscribe(channelName, updatedHandler as (message: unknown) => void); +}; + +/** + * Console-like API for publishing structured log events to the diagnostics channel. + * + * @property debug Logs messages with 'debug' level. + * @property info Logs messages with 'info' level. + * @property warn Logs messages with 'warn' level. + * @property error Logs messages with 'error' level. + */ +const log = { + debug: (msg?: unknown, ...args: unknown[]) => { + const options = getLoggerOptions(); + + return publish('debug', options, msg, ...args); + }, + info: (msg?: unknown, ...args: unknown[]) => { + const options = getLoggerOptions(); + + return publish('info', options, msg, ...args); + }, + warn: (msg?: unknown, ...args: unknown[]) => { + const options = getLoggerOptions(); + + return publish('warn', options, msg, ...args); + }, + error: (msg?: unknown, ...args: unknown[]) => { + const options = getLoggerOptions(); + + return publish('error', options, msg, ...args); + } +}; + +/** + * Register a handler that writes formatted log lines to `process.stderr`. + * + * Writes strictly to stderr to avoid corrupting STDIO with stdout. + * + * @param {LoggingSession} options + * @param [formatter] - Optional custom formatter for log events. Default prints: `[LEVEL] msg ...args` + * @returns Unsubscribe function to remove the subscriber + */ +const registerStderrSubscriber = (options: LoggingSession, formatter?: (e: LogEvent) => string) => { + const format = formatter || ((event: LogEvent) => { + const eventLevel = `[${event.level.toUpperCase()}]`; + const message = event.msg || ''; + const rest = event?.args?.map(arg => { + try { + return typeof arg === 'string' ? arg : JSON.stringify(arg); + } catch { + return String(arg); + } + }).join(' ') || ''; + const separator = rest ? '\t:' : ''; + + return `${eventLevel}:\t${message}${separator}${rest}`.trim(); + }); + + return subscribeToChannel((event: LogEvent) => { + if (logSeverity(event.level) >= logSeverity(options.level)) { + process.stderr.write(`${format(event)}\n`); + } + }); +}; + +/** + * Creates a logger initialization function and supports registering logging subscribers. + * + * @param {LoggingSession} [options] + * @returns Unsubscribe function to remove all registered subscribers + */ +const createLogger = (options: LoggingSession = getLoggerOptions()) => { + const unsubscribeLoggerFuncs: (() => void | boolean)[] = []; + + if (options?.channelName && options?.stderr) { + unsubscribeLoggerFuncs.push(registerStderrSubscriber(options)); + } + + return () => unsubscribeLoggerFuncs.forEach(unsubscribe => unsubscribe()); +}; + +export { + LOG_LEVELS, + createLogger, + log, + logSeverity, + publish, + registerStderrSubscriber, + subscribeToChannel, + type LogEvent, + type LogLevel +}; diff --git a/src/options.context.ts b/src/options.context.ts index 0a5febf..4b0c2bf 100644 --- a/src/options.context.ts +++ b/src/options.context.ts @@ -1,6 +1,8 @@ import { AsyncLocalStorage } from 'node:async_hooks'; +import { randomUUID } from 'node:crypto'; import { type GlobalOptions } from './options'; -import { DEFAULT_OPTIONS } from './options.defaults'; +import { DEFAULT_OPTIONS, LOG_BASENAME, type LoggingSession, type DefaultOptions } from './options.defaults'; +import { mergeObjects, freezeObject, isPlainObject } from './server.helpers'; /** * AsyncLocalStorage instance for per-instance options @@ -13,12 +15,35 @@ const optionsContext = new AsyncLocalStorage(); /** * Set and freeze cloned options in the current async context. * - * @param {Partial} options - Options to set in context (merged with DEFAULT_OPTIONS) - * @returns {GlobalOptions} Cloned frozen options object + * - Applies a unique session ID and logging channel name + * - Certain settings are not allowed to be overridden by the caller to ensure consistency across instances + * + * @param {Partial} [options] - Optional options to set in context. Merged with DEFAULT_OPTIONS. + * @returns {GlobalOptions} Cloned frozen default options object with session. */ -const setOptions = (options: Partial): GlobalOptions => { - const merged = { ...DEFAULT_OPTIONS, ...options } as GlobalOptions; - const frozen = Object.freeze(structuredClone(merged)); +const setOptions = (options?: Partial): GlobalOptions => { + const base = mergeObjects(DEFAULT_OPTIONS, options, { allowNullValues: false, allowUndefinedValues: false }); + const sessionId = (process.env.NODE_ENV === 'local' && '1234d567-1ce9-123d-1413-a1234e56c789') || randomUUID(); + + const baseLogging = isPlainObject(base.logging) ? base.logging : DEFAULT_OPTIONS.logging; + const baseName = LOG_BASENAME; + const channelName = `${baseName}:${sessionId}`; + const merged: GlobalOptions = { + ...base, + sessionId, + logging: { + level: baseLogging.level, + stderr: baseLogging.stderr, + protocol: baseLogging.protocol, + transport: baseLogging.transport, + baseName, + channelName + }, + resourceMemoOptions: DEFAULT_OPTIONS.resourceMemoOptions, + toolMemoOptions: DEFAULT_OPTIONS.toolMemoOptions + }; + + const frozen = freezeObject(structuredClone(merged)); optionsContext.enterWith(frozen); @@ -45,21 +70,28 @@ const getOptions = (): GlobalOptions => { return setOptions({}); }; +/** + * Get logging options from the current context. + * + * @returns {LoggingSession} Logging options from context. + */ +const getLoggerOptions = (): LoggingSession => getOptions().logging; + /** * Run a function with specific options context. Useful for testing or programmatic usage. * * @param options - Options to use in context * @param callback - Function to apply options context against - * @returns {Promise} Result of function + * @returns Result of function */ -const runWithOptions = async ( +const runWithOptions = async ( options: GlobalOptions, - callback: () => Promise -): Promise => { - const frozen = Object.freeze(structuredClone(options)); + callback: () => TReturn | Promise +) => { + const frozen = freezeObject(structuredClone(options)); return optionsContext.run(frozen, callback); }; -export { getOptions, optionsContext, runWithOptions, setOptions }; +export { getOptions, getLoggerOptions, optionsContext, runWithOptions, setOptions }; diff --git a/src/options.defaults.ts b/src/options.defaults.ts index 38534c4..811adb1 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -2,11 +2,40 @@ import { basename, join } from 'node:path'; import packageJson from '../package.json'; /** - * Application defaults (not user-configurable) + * Application defaults, not user-configurable + * + * @interface DefaultOptions + * + * @template TLogOptions The logging options type, defaulting to LoggingOptions. + * @property contextPath - Current working directory. + * @property docsHost - Flag indicating whether to use the docs-host. + * @property docsPath - Path to the documentation directory. + * @property llmsFilesPath - Path to the LLMs files directory. + * @property {LoggingOptions} logging - Logging options. + * @property name - Name of the package. + * @property repoName - Name of the repository. + * @property pfExternal - PatternFly external docs URL. + * @property pfExternalDesignComponents - PatternFly design guidelines' components' URL. + * @property pfExternalExamplesComponents - PatternFly examples' core components' URL. + * @property pfExternalExamplesLayouts - PatternFly examples' core layouts' URL. + * @property pfExternalExamplesCharts - PatternFly examples' charts' components' URL.' + * @property pfExternalExamplesTable - PatternFly examples' table components' URL. + * @property pfExternalChartsDesign - PatternFly charts' design guidelines URL. + * @property pfExternalDesignLayouts - PatternFly design guidelines' layouts' URL. + * @property pfExternalAccessibility - PatternFly accessibility URL. + * @property {typeof RESOURCE_MEMO_OPTIONS} resourceMemoOptions - Resource-level memoization options. + * @property {typeof TOOL_MEMO_OPTIONS} toolMemoOptions - Tool-specific memoization options. + * @property separator - Default string delimiter. + * @property urlRegex - Regular expression pattern for URL matching. + * @property version - Version of the package. */ -interface DefaultOptions { - resourceMemoOptions: Partial; - toolMemoOptions: Partial; +interface DefaultOptions { + contextPath: string; + docsHost: boolean; + docsPath: string; + llmsFilesPath: string; + logging: TLogOptions; + name: string; pfExternal: string; pfExternalDesignComponents: string; pfExternalExamplesComponents: string; @@ -16,16 +45,69 @@ interface DefaultOptions { pfExternalChartsDesign: string; pfExternalDesignLayouts: string; pfExternalAccessibility: string; + repoName: string | undefined; + resourceMemoOptions: Partial; separator: string; + toolMemoOptions: Partial; urlRegex: RegExp; - name: string; version: string; - repoName: string | undefined; - contextPath: string; - docsPath: string; - llmsFilesPath: string; } +/** + * Session defaults, not user-configurable + */ +/** + * Represents the default session configuration with an associated session ID, + * inheriting properties from the DefaultOptions interface. + * + * @extends DefaultOptions + * @property sessionId The unique identifier for the session. + */ +interface DefaultSession extends DefaultOptions { + readonly sessionId: string; +} + +/** + * Logging options. + * + * @interface LoggingOptions + * @default { level: 'info', stderr: false, protocol: false, baseName: `${packageJson.name}:log`, transport: 'stdio' } + * + * @property level Logging level. + * @property stderr Flag indicating whether to log to stderr. + * @property protocol Flag indicating whether to log protocol details. + * @property transport Transport mechanism for logging. + */ +interface LoggingOptions { + level: 'debug' | 'info' | 'warn' | 'error'; + stderr: boolean; + protocol: boolean; + transport: 'stdio' | 'mcp'; +} + +/** + * Logging session options, non-configurable by the user. + * + * @interface LoggingSession + * @extends LoggingOptions + * @property baseName Name of the logging channel. + * @property channelName Unique identifier for the logging channel. + */ +interface LoggingSession extends LoggingOptions { + readonly baseName: string; + readonly channelName: string; +} + +/** + * Base logging options. + */ +const LOGGING_OPTIONS: LoggingOptions = { + level: 'info', + stderr: false, + protocol: false, + transport: 'stdio' +}; + /** * Default separator for joining multiple document contents */ @@ -63,6 +145,11 @@ const TOOL_MEMO_OPTIONS = { } }; +/** + * Base logging channel name. Fixed to avoid user override. + */ +const LOG_BASENAME = 'pf-mcp:log'; + /** * URL regex pattern for detecting external URLs */ @@ -133,29 +220,15 @@ const PF_EXTERNAL_CHARTS_DESIGN = `${PF_EXTERNAL}/design-guidelines/charts`; /** * Global default options. Base defaults before CLI/programmatic overrides. * - * @type {GlobalOptions} Default options object. - * @property {CliOptions.docsHost} [docsHost] - Flag indicating whether to use the docs-host. - * @property {string} pfExternal - PatternFly external URL. - * @property {string} pfExternalDesignComponents - PatternFly design guidelines' components' URL. - * @property {string} pfExternalExamplesComponents - PatternFly examples' core components' URL. - * @property {string} pfExternalExamplesLayouts - PatternFly examples' core layouts' URL. - * @property {string} pfExternalExamplesCharts - PatternFly examples' charts' components' URL.' - * @property {string} pfExternalExamplesTable - PatternFly examples' table components' URL. - * @property {string} pfExternalChartsDesign - PatternFly charts' design guidelines URL. - * @property {string} pfExternalDesignLayouts - PatternFly design guidelines' layouts' URL. - * @property {string} pfExternalAccessibility - PatternFly accessibility URL. - * @property {typeof RESOURCE_MEMO_OPTIONS} resourceMemoOptions - Resource-level memoization options. - * @property {typeof TOOL_MEMO_OPTIONS} toolMemoOptions - Tool-specific memoization options. - * @property {string} separator - Default string delimiter. - * @property {RegExp} urlRegex - Regular expression pattern for URL matching. - * @property {string} name - Name of the package. - * @property {string} version - Version of the package. - * @property {string} repoName - Name of the repository. - * @property {string} contextPath - Current working directory. - * @property {string} docsPath - Path to the documentation directory. - * @property {string} llmsFilesPath - Path to the LLMs files directory. + * @type {DefaultOptions} Default options object. */ const DEFAULT_OPTIONS: DefaultOptions = { + docsHost: false, + contextPath: (process.env.NODE_ENV === 'local' && '/') || process.cwd(), + docsPath: (process.env.NODE_ENV === 'local' && '/documentation') || join(process.cwd(), 'documentation'), + llmsFilesPath: (process.env.NODE_ENV === 'local' && '/llms-files') || join(process.cwd(), 'llms-files'), + logging: LOGGING_OPTIONS, + name: packageJson.name, pfExternal: PF_EXTERNAL, pfExternalDesignComponents: PF_EXTERNAL_DESIGN_COMPONENTS, pfExternalExamplesComponents: PF_EXTERNAL_EXAMPLES_REACT_CORE, @@ -166,18 +239,14 @@ const DEFAULT_OPTIONS: DefaultOptions = { pfExternalDesignLayouts: PF_EXTERNAL_DESIGN_LAYOUTS, pfExternalAccessibility: PF_EXTERNAL_ACCESSIBILITY, resourceMemoOptions: RESOURCE_MEMO_OPTIONS, + repoName: basename(process.cwd() || '').trim(), toolMemoOptions: TOOL_MEMO_OPTIONS, separator: DEFAULT_SEPARATOR, urlRegex: URL_REGEX, - name: packageJson.name, - version: (process.env.NODE_ENV === 'local' && '0.0.0') || packageJson.version, - repoName: basename(process.cwd() || '').trim(), - contextPath: (process.env.NODE_ENV === 'local' && '/') || process.cwd(), - docsPath: (process.env.NODE_ENV === 'local' && '/documentation') || join(process.cwd(), 'documentation'), - llmsFilesPath: (process.env.NODE_ENV === 'local' && '/llms-files') || join(process.cwd(), 'llms-files') + version: (process.env.NODE_ENV === 'local' && '0.0.0') || packageJson.version }; -const DEFAULTS = { +export { PF_EXTERNAL, PF_EXTERNAL_VERSION, PF_EXTERNAL_EXAMPLES, @@ -190,31 +259,14 @@ const DEFAULTS = { PF_EXTERNAL_DESIGN_COMPONENTS, PF_EXTERNAL_DESIGN_LAYOUTS, PF_EXTERNAL_ACCESSIBILITY, - RESOURCE_MEMO_OPTIONS, - TOOL_MEMO_OPTIONS, + LOG_BASENAME, DEFAULT_OPTIONS, DEFAULT_SEPARATOR, - URL_REGEX -}; - -export { - DEFAULTS, - PF_EXTERNAL, - PF_EXTERNAL_VERSION, - PF_EXTERNAL_EXAMPLES, - PF_EXTERNAL_EXAMPLES_CHARTS, - PF_EXTERNAL_EXAMPLES_REACT_CORE, - PF_EXTERNAL_EXAMPLES_LAYOUTS, - PF_EXTERNAL_EXAMPLES_TABLE, - PF_EXTERNAL_EXAMPLES_VERSION, - PF_EXTERNAL_CHARTS_DESIGN, - PF_EXTERNAL_DESIGN_COMPONENTS, - PF_EXTERNAL_DESIGN_LAYOUTS, - PF_EXTERNAL_ACCESSIBILITY, RESOURCE_MEMO_OPTIONS, TOOL_MEMO_OPTIONS, - DEFAULT_OPTIONS, - DEFAULT_SEPARATOR, URL_REGEX, - type DefaultOptions + type DefaultOptions, + type DefaultSession, + type LoggingOptions, + type LoggingSession }; diff --git a/src/options.ts b/src/options.ts index c3ea32e..9ec3e28 100644 --- a/src/options.ts +++ b/src/options.ts @@ -1,31 +1,56 @@ -import { type DefaultOptions } from './options.defaults'; +import { DEFAULT_OPTIONS, type DefaultOptions, type DefaultSession, type LoggingOptions } from './options.defaults'; +import { type LogLevel, logSeverity } from './logger'; /** - * CLI options that users can set via command line arguments + * Combined options object */ -interface CliOptions { - docsHost?: boolean; - // Future CLI options can be added here -} +type GlobalOptions = DefaultSession; /** - * Combined options object + * Options parsed from CLI arguments */ -interface GlobalOptions extends CliOptions, DefaultOptions { - // Combined DefaultOptions and CliOptions -} +type CliOptions = { docsHost: boolean; logging: LoggingOptions }; /** - * Parse CLI arguments and return CLI options + * Parses CLI options and return config options for the application. + * + * Available options: + * - `--docs-host`: A flag indicating whether the documentation host should be enabled. + * - `--log-level `: Specifies the logging level. Valid values are `debug`, `info`, `warn`, and `error`. Defaults to `info`. + * - `--verbose`: Log all severity levels. Shortcut to set the logging level to `debug`. + * - `--log-stderr`: Enables terminal logging of channel events + * - `--log-protocol`: Enables MCP protocol logging. Forward server logs to MCP clients (requires advertising `capabilities.logging`). + * + * @param argv - Command-line arguments to parse. Defaults to `process.argv`. + * @returns Parsed command-line options. */ -const parseCliOptions = (): CliOptions => ({ - docsHost: process.argv.includes('--docs-host') - // Future CLI options can be added here -}); +const parseCliOptions = (argv: string[] = process.argv): CliOptions => { + const docsHost = argv.includes('--docs-host'); + const levelIndex = argv.indexOf('--log-level'); + + const logging: LoggingOptions = { + ...DEFAULT_OPTIONS.logging, + stderr: argv.includes('--log-stderr'), + protocol: argv.includes('--log-protocol') + }; + + if (argv.includes('--verbose')) { + logging.level = 'debug'; + } else if (levelIndex >= 0) { + const maybeLevel = String(argv[levelIndex + 1] || '').toLowerCase(); + + if (logSeverity(maybeLevel as LogLevel) > -1) { + logging.level = maybeLevel as LoggingOptions['level']; + } + } + + return { docsHost, logging }; +}; export { parseCliOptions, type CliOptions, + type LoggingOptions, type DefaultOptions, type GlobalOptions }; diff --git a/src/server.caching.ts b/src/server.caching.ts index 7b8f89b..d27f586 100644 --- a/src/server.caching.ts +++ b/src/server.caching.ts @@ -1,4 +1,5 @@ import { generateHash, isPromise } from './server.helpers'; +import { log } from './logger'; /** * Memo cache store. @@ -127,12 +128,12 @@ const memo = ( const cacheEntries = { remaining: [], removed: allCacheEntries, all: allCacheEntries }; if (isOnCacheExpirePromise) { - Promise.resolve(onCacheExpire?.(cacheEntries)).catch(console.error); + Promise.resolve(onCacheExpire?.(cacheEntries)).catch(error => log.error('onCacheExpire handler error', error)); } else { try { onCacheExpire?.(cacheEntries); } catch (error) { - console.error(error); + log.error('Memoized function error (uncached)', error); } } } @@ -210,12 +211,12 @@ const memo = ( const cacheEntries = { remaining: remainingCacheEntries, removed: removedCacheEntries, all: allCacheEntries }; if (isOnCacheRolloutPromise) { - Promise.resolve(onCacheRollout?.(cacheEntries)).catch(console.error); + Promise.resolve(onCacheRollout?.(cacheEntries)).catch(error => log.error('onCacheRollout handler error', error)); } else { try { onCacheRollout?.(cacheEntries); } catch (error) { - console.error(error); + log.error('Memoized function error (rolled out)', error); } } } diff --git a/src/server.helpers.ts b/src/server.helpers.ts index 4bd7397..87ed8d6 100644 --- a/src/server.helpers.ts +++ b/src/server.helpers.ts @@ -6,7 +6,7 @@ import { createHash, type BinaryToTextEncoding } from 'node:crypto'; * @param obj - Object, or otherwise, to check * @returns `true` if an object is an object */ -const isObject = (obj: unknown) => +const isObject = (obj: unknown): obj is Record => Object.prototype.toString.call(obj) === '[object Object]'; /** @@ -15,7 +15,7 @@ const isObject = (obj: unknown) => * @param obj - Object, or otherwise, to check * @returns `true` if an object is a "plain object" */ -const isPlainObject = (obj: unknown) => { +const isPlainObject = (obj: unknown): obj is Record => { if (!isObject(obj)) { return false; } @@ -25,6 +25,116 @@ const isPlainObject = (obj: unknown) => { return proto === null || proto === Object.prototype; }; +/** + * Merge two objects recursively, then return a new object, deep merge. + * + * Only recurses into plain objects. Arrays and non-plain objects are replaced, not merged. + * Prototype-pollution keys are ignored. + * + * @param baseObj - Base object to merge into + * @param sourceObj - Source object to merge from + * @param [options] - Merge options + * @param [options.allowNullValues] - If `true`, `null` values in `sourceObj` will overwrite `baseObj` values. Default: `true` + * @param [options.allowUndefinedValues] - If `true`, all undefined values in `sourceObj` will be merged on top of `baseObj`. Default: `false` + * @returns Deeply merged object of type TBase + */ +const mergeObjects = ( + baseObj: TBase, + sourceObj?: Partial | null, + { + allowNullValues = true, + allowUndefinedValues = false + }: { allowNullValues?: boolean; allowUndefinedValues?: boolean } = {} +): TBase => { + if (!isPlainObject(baseObj) || !isPlainObject(sourceObj)) { + return structuredClone(baseObj); + } + const pollutionKeys = ['__proto__', 'prototype', 'constructor']; + const result = { ...baseObj } as Record; + const src = sourceObj as Record; + + for (const key in src) { + if (!pollutionKeys.includes(key) && Object.hasOwn(src, key)) { + const baseVal = result[key]; + const srcVal = src[key]; + + if (!allowNullValues && srcVal === null) { + continue; + } + + if (!allowUndefinedValues && typeof srcVal === 'undefined') { + continue; + } + + if (isPlainObject(baseVal) && isPlainObject(srcVal)) { + result[key] = mergeObjects( + baseVal as object, + srcVal as object, + { allowNullValues, allowUndefinedValues } + ); + } else { + result[key] = srcVal; + } + } + } + + return result as TBase; +}; + +/** + * Freeze an object recursively, deep freeze. + * + * @param obj - Object to freeze + * @param [_seen] - WeakSet of already-seen objects. Default: `new WeakSet()`. + * @returns Frozen object of type TBase + */ +const freezeObject = (obj: TBase, _seen?: WeakSet): TBase => { + const seen = _seen || new WeakSet(); + + const queue: unknown[] = []; + const setQueue = (val: unknown) => { + if (val && (typeof val === 'object' || typeof val === 'function')) { + if (!seen.has(val)) { + seen.add(val); + queue.push(val); + } + } + }; + + setQueue(obj); + + while (queue.length) { + const current = queue.pop(); + + if (Object.isFrozen(current)) { + continue; + } + + if (Array.isArray(current)) { + for (const item of current) { + setQueue(item); + } + Object.freeze(current); + continue; + } + + if (isPlainObject(current)) { + for (const key of Object.keys(current)) { + setQueue(current[key]); + } + Object.freeze(current); + continue; + } + + // For non-plain objects (Map, Set, Date, RegExp, Function, etc.), + try { + Object.freeze(current); + } catch {} + } + + return obj; +}; + /** * Check if "is a Promise", "Promise like". * @@ -51,6 +161,7 @@ const hashCode = (str: unknown, { algorithm = 'sha1', encoding = 'hex' }: { algo * Normalize a value for hashing with JSON.stringify * * @param value - Value to normalize for hashing, typically for JSON.stringify + * @returns Normalized value suitable for JSON serialization */ const hashNormalizeValue = (value: unknown): unknown => { const normalizeSort = (a: any, b: any) => (a < b ? -1 : a > b ? 1 : 0); @@ -156,10 +267,12 @@ const generateHash = (anyValue: unknown): string => { }; export { + freezeObject, generateHash, hashCode, hashNormalizeValue, isObject, isPlainObject, - isPromise + isPromise, + mergeObjects }; diff --git a/src/server.logger.ts b/src/server.logger.ts new file mode 100644 index 0000000..2f7272d --- /dev/null +++ b/src/server.logger.ts @@ -0,0 +1,83 @@ +import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { type LoggingLevel } from '@modelcontextprotocol/sdk/types.js'; +import { getOptions } from './options.context'; +import { type GlobalOptions } from './options'; +import { createLogger, logSeverity, subscribeToChannel, type LogEvent, type LogLevel } from './logger'; +import { memo } from './server.caching'; + +type McpLoggingLevel = LoggingLevel; + +/** + * Convert a log level to an MCP-compatible level. + * + * @param {LogLevel} level - Log level to convert + * @returns MCP-compatible logging level + */ +const toMcpLevel = (level: LogLevel): McpLoggingLevel => { + switch (level) { + case 'debug': + return 'debug'; + case 'info': + return 'info'; + case 'warn': + return 'warning'; + case 'error': + default: + return 'error'; + } +}; + +/** + * Register a handler that forwards log events to connected MCP clients. + * + * - This requires the server to advertise `capabilities.logging`. + * - Event is fire-and-forget, swallow errors to avoid affecting app flow + * + * @param {McpServer} server - MCP server instance + * @param {GlobalOptions} options + * @returns Unsubscribe function to remove the subscriber. + */ +const registerMcpSubscriber = (server: McpServer, { logging, name }: GlobalOptions) => + subscribeToChannel((event: LogEvent) => { + if (logSeverity(event.level) < logSeverity(logging?.level)) { + return; + } + + const updatedMsg = event.msg && event?.args?.length ? { message: event.msg, args: event.args } : undefined; + const data = updatedMsg || event.msg || event.args || {}; + + try { + void server + .sendLoggingMessage({ level: toMcpLevel(event.level), logger: name, data }) + .catch(() => {}); + } catch {} + }); + +/** + * Create a logger for the server instance. + * + * @param {McpServer} server + * @param {GlobalOptions} options + * @returns Unsubscribe function to remove all registered subscribers + */ +const createServerLogger = (server: McpServer, options: GlobalOptions = getOptions()) => { + const unsubscribeLoggerFuncs: (() => boolean | void)[] = []; + + if (options?.logging?.channelName) { + // Register the diagnostics channel returns a function to unsubscribe + unsubscribeLoggerFuncs.push(createLogger(options.logging)); + + if (options.logging.protocol) { + unsubscribeLoggerFuncs.push(registerMcpSubscriber(server, options)); + } + } + + return () => unsubscribeLoggerFuncs.forEach(unsubscribe => unsubscribe()); +}; + +/** + * Memoize the server logger. + */ +createServerLogger.memo = memo(createServerLogger, { cacheLimit: 10 }); + +export { createServerLogger, registerMcpSubscriber, toMcpLevel, type McpLoggingLevel }; diff --git a/src/server.ts b/src/server.ts index a8b5493..1fa720d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -5,6 +5,8 @@ import { fetchDocsTool } from './tool.fetchDocs'; import { componentSchemasTool } from './tool.componentSchemas'; import { getOptions, runWithOptions } from './options.context'; import { type GlobalOptions } from './options'; +import { log } from './logger'; +import { createServerLogger } from './server.logger'; type McpTool = [string, { description: string; inputSchema: any }, (args: any) => Promise]; @@ -29,10 +31,10 @@ interface ServerInstance { /** * Create and run a server with shutdown, register tool and errors. * - * @param options - * @param settings - * @param settings.tools - * @param settings.enableSigint + * @param [options] + * @param [settings] + * @param [settings.tools] + * @param [settings.enableSigint] */ const runServer = async (options = getOptions(), { tools = [ @@ -44,18 +46,22 @@ const runServer = async (options = getOptions(), { }: { tools?: McpToolCreator[]; enableSigint?: boolean } = {}): Promise => { let server: McpServer | null = null; let transport: StdioServerTransport | null = null; + let unsubscribeServerLogger: (() => void) | null = null; let running = false; const stopServer = async () => { if (server && running) { await server?.close(); running = false; - console.log('PatternFly MCP server stopped'); + log.info('PatternFly MCP server stopped'); + unsubscribeServerLogger?.(); process.exit(0); } }; try { + const enableProtocolLogging = options?.logging?.protocol; + server = new McpServer( { name: options.name, @@ -63,15 +69,18 @@ const runServer = async (options = getOptions(), { }, { capabilities: { - tools: {} + tools: {}, + ...(enableProtocolLogging ? { logging: {} } : {}) } } ); + unsubscribeServerLogger = createServerLogger.memo(server); + tools.forEach(toolCreator => { const [name, schema, callback] = toolCreator(options); - console.info(`Registered tool: ${name}`); + log.info(`Registered tool: ${name}`); server?.registerTool(name, schema, (args = {}) => runWithOptions(options, async () => await callback(args))); }); @@ -84,9 +93,9 @@ const runServer = async (options = getOptions(), { await server.connect(transport); running = true; - console.log('PatternFly MCP server running on stdio'); + log.info('PatternFly MCP server running on stdio'); } catch (error) { - console.error('Error creating MCP server:', error); + log.error('Error creating MCP server:', error); throw error; } diff --git a/tests/__snapshots__/stdioTransport.test.ts.snap b/tests/__snapshots__/stdioTransport.test.ts.snap index 6519e8a..cbfe25f 100644 --- a/tests/__snapshots__/stdioTransport.test.ts.snap +++ b/tests/__snapshots__/stdioTransport.test.ts.snap @@ -274,6 +274,49 @@ exports[`Hosted mode, --docs-host should read llms-files and includes expected t ] `; +exports[`Logging should allow setting logging options, default 1`] = `[]`; + +exports[`Logging should allow setting logging options, stderr 1`] = ` +[ + "[INFO]: Registered tool: usePatternFlyDocs +", + "[INFO]: Registered tool: fetchDocs +", + "[INFO]: Registered tool: componentSchemas +", + "[INFO]: PatternFly MCP server running on stdio +", +] +`; + +exports[`Logging should allow setting logging options, verbose 1`] = ` +[ + "[INFO]: Registered tool: usePatternFlyDocs +", + "[INFO]: Registered tool: fetchDocs +", + "[INFO]: Registered tool: componentSchemas +", + "[INFO]: PatternFly MCP server running on stdio +", +] +`; + +exports[`Logging should allow setting logging options, with log level filtering 1`] = `[]`; + +exports[`Logging should allow setting logging options, with mcp protocol 1`] = ` +[ + { + "method": "notifications/message", + "params": { + "data": "PatternFly MCP server running on stdio", + "level": "info", + "logger": "@patternfly/patternfly-mcp", + }, + }, +] +`; + exports[`PatternFly MCP, STDIO should concatenate headers and separator with two local files 1`] = ` "# Documentation from documentation/guidelines/README.md diff --git a/tests/stdioTransport.test.ts b/tests/stdioTransport.test.ts index 9bb6e7a..23ca211 100644 --- a/tests/stdioTransport.test.ts +++ b/tests/stdioTransport.test.ts @@ -71,6 +71,38 @@ describe('Hosted mode, --docs-host', () => { }); }); +describe('Logging', () => { + it.each([ + { + description: 'default', + args: [] + }, + { + description: 'stderr', + args: ['--log-stderr'] + }, + { + description: 'verbose', + args: ['--log-stderr', '--verbose'] + }, + { + description: 'with log level filtering', + args: ['--log-level', 'warn'] + }, + { + description: 'with mcp protocol', + args: ['--verbose', '--log-protocol'] + } + ])('should allow setting logging options, $description', async ({ args }) => { + const serverArgs = [...args]; + const client = await startServer({ args: serverArgs }); + + expect(client.logs()).toMatchSnapshot(); + + await client.stop(); + }); +}); + describe('External URLs', () => { let fetchMock: Awaited> | undefined; let url: string; diff --git a/tests/utils/stdioTransportClient.ts b/tests/utils/stdioTransportClient.ts index 8e231d5..73291a9 100644 --- a/tests/utils/stdioTransportClient.ts +++ b/tests/utils/stdioTransportClient.ts @@ -4,7 +4,8 @@ */ import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; -import { ResultSchema } from '@modelcontextprotocol/sdk/types.js'; +import { ResultSchema, LoggingMessageNotificationSchema, type LoggingLevel } from '@modelcontextprotocol/sdk/types.js'; +import { parseCliOptions } from '../../src/options'; export interface StartOptions { command?: string; @@ -28,6 +29,9 @@ export interface StdioTransportClient { send: (request: { method: string; params?: any }, opts?: { timeoutMs?: number }) => Promise; stop: (signal?: NodeJS.Signals) => Promise; close: () => Promise; // Alias for stop() + logs: () => string[]; + stderrLogs: () => string[]; + protocolLogs: () => string[]; } /** @@ -38,7 +42,7 @@ export interface StdioTransportClient { * @param options - Server configuration options * @param options.command - Node command to run (default: 'node') * @param options.serverPath - Path to built server (default: process.env.SERVER_PATH || 'dist/index.js') - * @param options.args - Additional args to pass to server (e.g., ['--docs-host']) + * @param options.args - Additional args to pass to server, see app `CliOptions` for the full list (e.g., ['--docs-host']) * @param options.env - Environment variables for the child process */ export const startServer = async ({ @@ -67,6 +71,9 @@ export const startServer = async ({ } ); + const parsedArgs = parseCliOptions(args); + const loggingArgs = parsedArgs?.logging || {}; + // Track whether we're intentionally closing the client // This allows us to suppress expected errors during cleanup let isClosing = false; @@ -88,15 +95,35 @@ export const startServer = async ({ } }; - // Connect client to transport (this automatically starts transport and initializes the session) + // Collect protocol logs (MCP notifications/message) when enabled via CLI arg + const protocolLogs: any[] = []; + + // Register the handler BEFORE connect so we don't miss early server messages + if (loggingArgs.protocol) { + try { + mcpClient.setNotificationHandler(LoggingMessageNotificationSchema, (params: any) => { + protocolLogs.push(params); + }); + } catch {} + } + + // Connect client to transport. This automatically starts transport and initializes the session await mcpClient.connect(transport); - // Access stderr stream if available to handle server logs - // This prevents server logs from interfering with JSON-RPC parsing + // Negotiate protocol logging level if the server advertises it + if (loggingArgs.protocol) { + try { + await mcpClient.setLoggingLevel(loggingArgs.level as LoggingLevel); + } catch {} + } + + // Access stderr stream if available. stderr is used to prevent logs from interfering with JSON-RPC parsing + // Collect server stderr logs + const stderrLogs: string[] = []; + if (transport.stderr) { - transport.stderr.on('data', (_data: Buffer) => { - // Server logs go to stderr, we can optionally log them for debugging, - // but we don't need to do anything with them for the tests to work + transport.stderr.on('data', (data: Buffer) => { + stderrLogs.push(data.toString()); }); } @@ -183,6 +210,12 @@ export const startServer = async ({ } }, + logs: () => [ + ...stderrLogs, + ...protocolLogs + ], + stderrLogs: () => stderrLogs.slice(), + protocolLogs: () => protocolLogs.slice(), stop, close: stop };