From 0f4427804f5d793f7c21e0688ef1abfd5065d227 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Wed, 4 Feb 2026 12:23:41 -0500 Subject: [PATCH 1/3] refactor(options): mode options for e2e testing * index, refactor modes to account for cli vs programmatic adds * options, mode, modeOptions added with focus on test mode * server.getResources, patch path checks, mock redirects for testing * testing, unified mocks for e2e stdio and http transport using test mode --- .../options.defaults.test.ts.snap | 20 +++ .../__snapshots__/options.test.ts.snap | 60 ++++++++ .../server.getResources.test.ts.snap | 6 +- src/__tests__/index.test.ts | 12 +- src/__tests__/server.getResources.test.ts | 41 +++++- src/index.ts | 64 +++++---- src/options.context.ts | 9 +- src/options.defaults.ts | 133 +++++++++++++++--- src/options.ts | 42 +++++- src/server.getResources.ts | 107 ++++++++++++-- .../__snapshots__/httpTransport.test.ts.snap | 122 +++------------- .../__snapshots__/stdioTransport.test.ts.snap | 110 +++------------ tests/httpTransport.test.ts | 22 +-- tests/stdioTransport.test.ts | 27 +++- tests/utils/fetchMock.ts | 17 ++- tests/utils/httpTransportClient.ts | 3 +- tests/utils/stdioTransportClient.ts | 2 +- 17 files changed, 506 insertions(+), 291 deletions(-) diff --git a/src/__tests__/__snapshots__/options.defaults.test.ts.snap b/src/__tests__/__snapshots__/options.defaults.test.ts.snap index d9725955..973ea9f1 100644 --- a/src/__tests__/__snapshots__/options.defaults.test.ts.snap +++ b/src/__tests__/__snapshots__/options.defaults.test.ts.snap @@ -5,6 +5,7 @@ exports[`options defaults should return specific properties: defaults 1`] = ` "contextPath": "/", "contextUrl": "file:///", "docsPath": "/documentation", + "docsPathSlug": "documentation:", "http": { "allowedHosts": [], "allowedOrigins": [], @@ -21,8 +22,27 @@ exports[`options defaults should return specific properties: defaults 1`] = ` }, "maxDocsToLoad": 500, "maxSearchLength": 256, + "mode": "programmatic", + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "name": "@patternfly/patternfly-mcp", "nodeVersion": 22, + "patternflyOptions": { + "availableResourceVersions": [ + "v6", + ], + "default": { + "defaultVersion": "v6", + "versionStrategy": "highest", + "versionWhitelist": [ + "@patternfly/react-core", + "@patternfly/patternfly", + ], + }, + }, "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", diff --git a/src/__tests__/__snapshots__/options.test.ts.snap b/src/__tests__/__snapshots__/options.test.ts.snap index 438fddf8..03c577f3 100644 --- a/src/__tests__/__snapshots__/options.test.ts.snap +++ b/src/__tests__/__snapshots__/options.test.ts.snap @@ -16,6 +16,11 @@ exports[`parseCliOptions should attempt to parse args with --allowed-hosts 1`] = "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -37,6 +42,11 @@ exports[`parseCliOptions should attempt to parse args with --allowed-origins 1`] "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -55,6 +65,11 @@ exports[`parseCliOptions should attempt to parse args with --http and --host 1`] "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -73,6 +88,11 @@ exports[`parseCliOptions should attempt to parse args with --http and --port 1`] "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -91,6 +111,11 @@ exports[`parseCliOptions should attempt to parse args with --http and invalid -- "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -107,6 +132,11 @@ exports[`parseCliOptions should attempt to parse args with --http flag 1`] = ` "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -123,6 +153,11 @@ exports[`parseCliOptions should attempt to parse args with --log-level flag 1`] "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -139,6 +174,11 @@ exports[`parseCliOptions should attempt to parse args with --log-stderr flag and "stderr": true, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -155,6 +195,11 @@ exports[`parseCliOptions should attempt to parse args with --tool 1`] = ` "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [ "my-tool", @@ -174,6 +219,11 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] = "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -190,6 +240,11 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag and -- "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } @@ -206,6 +261,11 @@ exports[`parseCliOptions should attempt to parse args with other arguments 1`] = "stderr": false, "transport": "stdio", }, + "modeOptions": { + "cli": {}, + "programmatic": {}, + "test": {}, + }, "pluginIsolation": undefined, "toolModules": [], } diff --git a/src/__tests__/__snapshots__/server.getResources.test.ts.snap b/src/__tests__/__snapshots__/server.getResources.test.ts.snap index 6f9d23e2..4ebe25d7 100644 --- a/src/__tests__/__snapshots__/server.getResources.test.ts.snap +++ b/src/__tests__/__snapshots__/server.getResources.test.ts.snap @@ -91,7 +91,11 @@ exports[`promiseQueue should execute promises in order: allSettled 1`] = ` ] `; -exports[`resolveLocalPathFunction should return a consistent path, basic 1`] = `"/lorem-ipsum.md"`; +exports[`resolveLocalPathFunction should return a consistent path, basic 1`] = `"/app/project/lorem-ipsum.md"`; + +exports[`resolveLocalPathFunction should return a consistent path, documentation slug 1`] = `"/documentation/guidelines/README.md"`; + +exports[`resolveLocalPathFunction should return a consistent path, relative path with valid navigation 1`] = `"/app/project/file.md"`; exports[`resolveLocalPathFunction should return a consistent path, url, file 1`] = `"file://someDirectory/dolor-sit.md"`; diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 37f60bf3..05288983 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -79,7 +79,7 @@ describe('main', () => { mockRunServer.mockRejectedValue(error); - await main(); + await expect(async () => main()).rejects.toThrow(error.message); expect(consoleErrorSpy).toHaveBeenCalledWith('Failed to start server:', error); expect(processExitSpy).toHaveBeenCalledWith(1); @@ -89,25 +89,25 @@ describe('main', () => { { description: 'parseCliOptions', error: new Error('Failed to parse CLI options'), - message: 'Failed to start server:', + message: 'Set options error, failed to start server:', method: main }, { description: 'setOptions', error: new Error('Failed to set options'), - message: 'Failed to start server:', + message: 'Set options error, failed to start server:', method: main }, { description: 'parseCliOptions, with start alias', error: new Error('Failed to parse CLI options'), - message: 'Failed to start server:', + message: 'Set options error, failed to start server:', method: start }, { description: 'setOptions, with start alias', error: new Error('Failed to set options'), - message: 'Failed to start server:', + message: 'Set options error, failed to start server:', method: start } ])('should handle errors, $description', async ({ error, message, method }) => { @@ -115,7 +115,7 @@ describe('main', () => { throw error; }); - await method(); + await expect(async () => method()).rejects.toThrow(error.message); expect(consoleErrorSpy).toHaveBeenCalledWith(message, error); expect(processExitSpy).toHaveBeenCalledWith(1); diff --git a/src/__tests__/server.getResources.test.ts b/src/__tests__/server.getResources.test.ts index 3de885fd..42709a99 100644 --- a/src/__tests__/server.getResources.test.ts +++ b/src/__tests__/server.getResources.test.ts @@ -8,6 +8,7 @@ import { resolveLocalPathFunction } from '../server.getResources'; import { type GlobalOptions } from '../options'; +import { DEFAULT_OPTIONS } from '../options.defaults'; // Mock dependencies jest.mock('node:fs/promises'); @@ -103,12 +104,45 @@ describe('resolveLocalPathFunction', () => { { description: 'url, file', path: 'file://someDirectory/dolor-sit.md' + }, + { + description: 'documentation slug', + path: 'documentation:guidelines/README.md' + }, + { + description: 'relative path with valid navigation', + path: './subdir/../file.md' } ])('should return a consistent path, $description', ({ path }) => { - const result = resolveLocalPathFunction(path); + const result = resolveLocalPathFunction(path, { ...DEFAULT_OPTIONS, contextPath: '/app/project' }); expect(result).toMatchSnapshot(); }); + + it.each([ + { + description: 'sibling directory traversal attempt', + path: '../patternfly-mcp-secret/config.json', + shouldThrow: 'Access denied' + }, + { + description: 'absolute path outside base', + path: '/etc/passwd', + shouldThrow: 'Access denied' + }, + { + description: 'documentation traversal attempt', + path: 'documentation:../../etc/passwd', + shouldThrow: 'Access denied' + }, + { + description: 'path matching prefix but not boundary', + path: '../project-sibling/file.txt', + shouldThrow: 'Access denied' + } + ])('should return a consistent path or throw, $description', ({ path, shouldThrow }) => { + expect(() => resolveLocalPathFunction(path, { ...DEFAULT_OPTIONS, contextPath: '/app/project' })).toThrow(shouldThrow); + }); }); describe('loadFileFetch', () => { @@ -126,6 +160,11 @@ describe('loadFileFetch', () => { description: 'with remote URL', pathUrl: 'https://example.com/remote.md', expectedIsFetch: true + }, + { + description: 'with documentation slug', + pathUrl: 'documentation:guidelines/README.md', + expectedIsFetch: false } ])('should attempt to load a file or fetch, $description', async ({ pathUrl, expectedIsFetch }) => { const mockFetchCall = jest.fn().mockResolvedValue('content'); diff --git a/src/index.ts b/src/index.ts index 9bdbd2ac..fc24c20a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,7 +9,8 @@ import { type ServerLogEvent, type ServerStatReport, type ServerStats, - type ServerGetStats + type ServerGetStats, + type ServerOptions } from './server'; import { createMcpTool, @@ -23,16 +24,8 @@ import { /** * Options for "programmatic" use. Extends the `DefaultOptions` interface. - * - * @property {('cli' | 'programmatic' | 'test')} [mode] - Optional string property that specifies the mode of operation. - * Defaults to `'programmatic'`. - * - `'cli'`: Functionality is being executed in a cli context. Allows process exits. - * - `'programmatic'`: Functionality is invoked programmatically. Allows process exits. - * - `'test'`: Functionality is being tested. Does NOT allow process exits. */ -type PfMcpOptions = DefaultOptionsOverrides & { - mode?: 'cli' | 'programmatic' | 'test'; -}; +type PfMcpOptions = DefaultOptionsOverrides; /** * Additional settings for programmatic control. @@ -102,9 +95,8 @@ type PfMcpStatReport = ServerStatReport; * * @returns {Promise} Server-instance with shutdown capability * - * @throws {Error} If the server fails to start or any error occurs during initialization, - * and `allowProcessExit` is set to `false`, the error will be thrown rather than exiting - * the process. + * @throws {Error} If `allowProcessExit` is set to `false` an error will be thrown rather than exiting + * the process. Server errors are noted as options or start failures. * * @example Programmatic: A MCP server with STDIO (Standard Input Output) transport. * import { start } from '@patternfly/patternfly-mcp'; @@ -157,30 +149,46 @@ const main = async ( pfMcpOptions: PfMcpOptions = {}, pfMcpSettings: PfMcpSettings = {} ): Promise => { - const { mode, ...options } = pfMcpOptions; + const { mode: programmaticMode, ...options } = pfMcpOptions; const { allowProcessExit } = pfMcpSettings; - const modes = ['cli', 'programmatic', 'test']; - const updatedMode = mode && modes.includes(mode) ? mode : 'programmatic'; - const updatedAllowProcessExit = allowProcessExit ?? updatedMode !== 'test'; + // Check early for allowing process exits + let updatedAllowProcessExit = allowProcessExit ?? programmaticMode !== 'test'; + let mergedOptions: ServerOptions; + + // If allowed, exit the process on error + const processExit = (message: string, error: unknown) => { + console.error(message, error); + + if (updatedAllowProcessExit) { + process.exit(1); + } + }; try { - const cliOptions = parseCliOptions(); - const mergedOptions = setOptions({ ...cliOptions, ...options }); + // Parse CLI options + const { mode: cliMode, ...cliOptions } = parseCliOptions(); + + // Apply `mode` separately because `cli.ts` applies it programmatically. Doing this allows us to set mode through `CLI options`. + mergedOptions = setOptions({ ...cliOptions, ...options, mode: cliMode ?? programmaticMode }); + + // Finalize exit policy after merging options + updatedAllowProcessExit = allowProcessExit ?? mergedOptions.mode !== 'test'; + } catch (error) { + processExit('Set options error, failed to start server:', error); + throw error; + } + + try { + // Generate session options const session = getSessionOptions(); - // use runWithSession to enable session in listeners + // Start the server, apply session values, then apply merged options to ensure stable hashing. return await runWithSession(session, async () => - // `runServer` doesn't require options in the memo key, but we pass fully merged options for stable hashing await runServer.memo(mergedOptions, { allowProcessExit: updatedAllowProcessExit })); } catch (error) { - console.error('Failed to start server:', error); - - if (updatedAllowProcessExit) { - process.exit(1); - } else { - throw error; - } + processExit('Failed to start server:', error); + throw error; } }; diff --git a/src/options.context.ts b/src/options.context.ts index 8fe633c0..9d21ffe1 100644 --- a/src/options.context.ts +++ b/src/options.context.ts @@ -1,7 +1,13 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { randomUUID } from 'node:crypto'; import { type AppSession, type GlobalOptions, type DefaultOptionsOverrides } from './options'; -import { DEFAULT_OPTIONS, LOG_BASENAME, type LoggingSession, type StatsSession } from './options.defaults'; +import { + DEFAULT_OPTIONS, + LOG_BASENAME, + MODE_LEVELS, + type LoggingSession, + type StatsSession +} from './options.defaults'; import { mergeObjects, freezeObject, isPlainObject, hashCode } from './server.helpers'; /** @@ -83,6 +89,7 @@ const setOptions = (options?: DefaultOptionsOverrides): GlobalOptions => { const merged: GlobalOptions = { ...base, + mode: MODE_LEVELS.includes(base.mode) ? base.mode : DEFAULT_OPTIONS.mode, logging: { level: ['debug', 'info', 'warn', 'error'].includes(baseLogging.level) ? baseLogging.level : DEFAULT_OPTIONS.logging.level, logger: baseLogging.logger, diff --git a/src/options.defaults.ts b/src/options.defaults.ts index d41c4734..e92bdaaa 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -12,17 +12,18 @@ import { type ToolModule } from './server.toolsUser'; * @property contextPath - Current working directory. * @property contextUrl - Current working directory URL. * @property docsPath - Path to the documentation directory. + * @property docsPathSlug - Local docs slug. Used for resolving local stored documentation. * @property isHttp - Flag indicating whether the server is running in HTTP mode. * @property {HttpOptions} http - HTTP server options. * @property {LoggingOptions} logging - Logging options. * @property maxDocsToLoad - Maximum number of docs to load. * @property maxSearchLength - Maximum length for search strings. * @property recommendedMaxDocsToLoad - Recommended maximum number of docs to load. + * @property {typeof MODE_LEVELS} mode - Specifies the mode of operation. + * @property {ModeOptions} modeOptions - Mode-specific options. * @property name - Name of the package. * @property nodeVersion - Node.js major version. - * @property pluginIsolation - Isolation preset for external plugins. - * @property {PluginHostOptions} pluginHost - Plugin host options. - * @property repoName - Name of the repository. + * @property {PatternFlyOptions} patternflyOptions - PatternFly-specific options. * @property pfExternal - PatternFly external docs URL. * @property pfExternalDesignComponents - PatternFly design guidelines' components' URL. * @property pfExternalExamplesComponents - PatternFly examples' core components' URL. @@ -32,6 +33,9 @@ import { type ToolModule } from './server.toolsUser'; * @property pfExternalChartsDesign - PatternFly charts' design guidelines URL. * @property pfExternalDesignLayouts - PatternFly design guidelines' layouts' URL. * @property pfExternalAccessibility - PatternFly accessibility URL. + * @property pluginIsolation - Isolation preset for external plugins. + * @property {PluginHostOptions} pluginHost - Plugin host options. + * @property repoName - Name of the repository. * @property {typeof RESOURCE_MEMO_OPTIONS} resourceMemoOptions - Resource-level memoization options. * @property separator - Default string delimiter. * @property {StatsOptions} stats - Stats options. @@ -46,14 +50,18 @@ interface DefaultOptions { contextPath: string; contextUrl: string; docsPath: string; + docsPathSlug: string; http: HttpOptions; isHttp: boolean; logging: TLogOptions; maxDocsToLoad: number; maxSearchLength: number; recommendedMaxDocsToLoad: number; + mode: 'cli' | 'programmatic' | 'test'; + modeOptions: ModeOptions; name: string; nodeVersion: number; + patternflyOptions: PatternFlyOptions; pluginIsolation: 'none' | 'strict'; pluginHost: PluginHostOptions; pfExternal: string; @@ -78,17 +86,38 @@ interface DefaultOptions { } /** - * Overrides for default options. + * Overrides for default options. Exposed to the consumer/user. */ type DefaultOptionsOverrides = Partial< - Omit + Omit > & { + mode?: DefaultOptions['mode'] | undefined; + modeOptions?: Partial | undefined; http?: Partial; logging?: Partial; pluginIsolation?: 'none' | 'strict' | undefined; toolModules?: ToolModule | ToolModule[] | undefined; }; +/** + * HTTP server options. + * + * See `HTTP_OPTIONS` for defaults. + * + * @interface HttpOptions + * + * @property port Port number. + * @property host Host name. + * @property allowedOrigins List of allowed origins. + * @property allowedHosts List of allowed hosts. + */ +interface HttpOptions { + port: number; + host: string; + allowedOrigins: string[]; + allowedHosts: string[]; +} + /** * Logging options. * @@ -113,22 +142,38 @@ interface LoggingOptions { } /** - * HTTP server options. - * - * See `HTTP_OPTIONS` for defaults. + * Mode-specific options. * - * @interface HttpOptions + * @interface ModeOptions + * @property test Test-specific options. + * @property test.baseUrl Base URL for testing. + */ +interface ModeOptions { + cli?: object | undefined; + programmatic?: object | undefined; + test?: { + baseUrl?: string | undefined; + } | undefined; +} + +/** + * PatternFly-specific options. * - * @property port Port number. - * @property host Host name. - * @property allowedOrigins List of allowed origins. - * @property allowedHosts List of allowed hosts. - */ -interface HttpOptions { - port: number; - host: string; - allowedOrigins: string[]; - allowedHosts: string[]; + * @property availableResourceVersions List of intended available PatternFly resource versions to the MCP server. + * @property default Default specific options. + * @property default.defaultVersion Default PatternFly version. + * @property default.versionWhitelist List of mostly reliable dependencies to scan for when detecting the PatternFly version. + * @property default.versionStrategy Strategy to use when multiple PatternFly versions are detected. + * - 'highest': Use the highest major version found. + * - 'lowest': Use the lowest major version found. + */ +interface PatternFlyOptions { + availableResourceVersions: string[]; + default: { + defaultVersion: string; + versionWhitelist: string[]; + versionStrategy: 'highest' | 'lowest'; + } } /** @@ -219,6 +264,15 @@ const HTTP_OPTIONS: HttpOptions = { allowedHosts: [] }; +/** + * Mode-specific options. + */ +const MODE_OPTIONS: ModeOptions = { + cli: {}, + programmatic: {}, + test: {} +}; + /** * Default plugin host options. */ @@ -290,11 +344,37 @@ const XHR_FETCH_OPTIONS: XhrFetchOptions = { */ const LOG_BASENAME = 'pf-mcp:log'; +/** + * Default PatternFly-specific options. + */ +const PATTERNFLY_OPTIONS: PatternFlyOptions = { + // availableVersions: ['v3', 'v4', 'v5', 'v6'], + availableResourceVersions: ['v6'], + default: { + defaultVersion: 'v6', + versionWhitelist: [ + '@patternfly/react-core', + '@patternfly/patternfly' + ], + versionStrategy: 'highest' + } +}; + /** * URL regex pattern for detecting external URLs */ const URL_REGEX = /^(https?:)\/\//i; +/** + * Available operational modes for the MCP server. + * + * Each mode represents an operational domain: + * - `cli`: Command-line interface mode. + * - `programmatic`: Programmatic interaction mode where the application is used as a library or API. + * - `test`: Testing or debugging mode. + */ +const MODE_LEVELS: DefaultOptions['mode'][] = ['cli', 'programmatic', 'test']; + const PF_EXTERNAL_EXAMPLES_VERSION = 'v6.4.0'; /** @@ -386,16 +466,18 @@ const DEFAULT_OPTIONS: DefaultOptions = { contextPath: (process.env.NODE_ENV === 'local' && '/') || resolve(process.cwd()), contextUrl: pathToFileURL((process.env.NODE_ENV === 'local' && '/') || resolve(process.cwd())).href, docsPath: (process.env.NODE_ENV === 'local' && '/documentation') || join(resolve(process.cwd()), 'documentation'), + docsPathSlug: 'documentation:', isHttp: false, http: HTTP_OPTIONS, logging: LOGGING_OPTIONS, maxDocsToLoad: 500, maxSearchLength: 256, recommendedMaxDocsToLoad: 15, + mode: 'programmatic', + modeOptions: MODE_OPTIONS, name: packageJson.name, nodeVersion: (process.env.NODE_ENV === 'local' && 22) || getNodeMajorVersion(), - pluginIsolation: 'strict', - pluginHost: PLUGIN_HOST_OPTIONS, + patternflyOptions: PATTERNFLY_OPTIONS, pfExternal: PF_EXTERNAL, pfExternalDesignComponents: PF_EXTERNAL_DESIGN_COMPONENTS, pfExternalExamplesComponents: PF_EXTERNAL_EXAMPLES_REACT_CORE, @@ -405,6 +487,8 @@ const DEFAULT_OPTIONS: DefaultOptions = { pfExternalChartsDesign: PF_EXTERNAL_CHARTS_DESIGN, pfExternalDesignLayouts: PF_EXTERNAL_DESIGN_LAYOUTS, pfExternalAccessibility: PF_EXTERNAL_ACCESSIBILITY, + pluginIsolation: 'strict', + pluginHost: PLUGIN_HOST_OPTIONS, resourceMemoOptions: RESOURCE_MEMO_OPTIONS, repoName: basename(process.cwd() || '').trim(), stats: STATS_OPTIONS, @@ -418,6 +502,9 @@ const DEFAULT_OPTIONS: DefaultOptions = { }; export { + LOG_BASENAME, + DEFAULT_OPTIONS, + MODE_LEVELS, PF_EXTERNAL, PF_EXTERNAL_VERSION, PF_EXTERNAL_EXAMPLES, @@ -430,14 +517,14 @@ export { PF_EXTERNAL_DESIGN_COMPONENTS, PF_EXTERNAL_DESIGN_LAYOUTS, PF_EXTERNAL_ACCESSIBILITY, - LOG_BASENAME, - DEFAULT_OPTIONS, getNodeMajorVersion, type DefaultOptions, type DefaultOptionsOverrides, type HttpOptions, type LoggingOptions, type LoggingSession, + type ModeOptions, + type PatternFlyOptions, type PluginHostOptions, type StatsSession, type XhrFetchOptions diff --git a/src/options.ts b/src/options.ts index 8da94c2d..17389172 100644 --- a/src/options.ts +++ b/src/options.ts @@ -1,6 +1,14 @@ -import { DEFAULT_OPTIONS, type DefaultOptions, type DefaultOptionsOverrides, type LoggingOptions, type HttpOptions } from './options.defaults'; +import { + DEFAULT_OPTIONS, + MODE_LEVELS, + type DefaultOptions, + type DefaultOptionsOverrides, + type LoggingOptions, + type HttpOptions, + type ModeOptions +} from './options.defaults'; import { type LogLevel, logSeverity } from './logger'; -import { portValid } from './server.helpers'; +import { isUrl, portValid } from './server.helpers'; /** * Session defaults, not user-configurable @@ -20,6 +28,8 @@ type GlobalOptions = DefaultOptions; * Options parsed from CLI arguments */ type CliOptions = { + mode?: DefaultOptions['mode']; + modeOptions?: Partial; http?: Partial; isHttp: boolean; logging: Partial; @@ -70,6 +80,8 @@ const getArgValue = (flag: string, { defaultValue, argv = process.argv }: { defa * Parses CLI options and return config options for the application. * * Available options: + * - `--mode `: Specifies the mode of operation. Valid values are `cli`, `programmatic`, and `test`. + * - `--mode-test-url`: Specifies the base URL for testing mode. * - `--log-level `: Specifies the logging level. Valid values are `debug`, `info`, `warn`, and `error`. * - `--verbose`: Log all severity levels. Shortcut to set the logging level to `debug`. * - `--log-stderr`: Enables terminal logging of channel events @@ -87,6 +99,11 @@ const getArgValue = (flag: string, { defaultValue, argv = process.argv }: { defa * @returns Parsed command-line options. */ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { + const modeIndex = argv.indexOf('--mode'); + const modeTestUrl = argv.indexOf('--mode-test-url'); + const modeOptions: ModeOptions = { + ...DEFAULT_OPTIONS.modeOptions + }; const levelIndex = argv.indexOf('--log-level'); const logging: LoggingOptions = { ...DEFAULT_OPTIONS.logging, @@ -94,6 +111,25 @@ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { protocol: argv.includes('--log-protocol') }; + let mode: CliOptions['mode'] | undefined; + + if (modeIndex >= 0) { + const maybeMode = String(argv[modeIndex + 1] || '').toLowerCase(); + + if (MODE_LEVELS.includes(maybeMode as DefaultOptions['mode'])) { + mode = argv[modeIndex + 1] as DefaultOptions['mode']; + } + } + + if (modeTestUrl >= 0) { + const maybeBaseUrl = String(argv[modeTestUrl + 1] || '').trim(); + + if (isUrl(maybeBaseUrl)) { + modeOptions.test ??= {}; + modeOptions.test.baseUrl = maybeBaseUrl; + } + } + if (argv.includes('--verbose')) { logging.level = 'debug'; } else if (levelIndex >= 0) { @@ -189,6 +225,8 @@ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { } return { + ...(mode ? { mode } : {}), + modeOptions, logging, isHttp, http, diff --git a/src/server.getResources.ts b/src/server.getResources.ts index 2e0225fd..d6469297 100644 --- a/src/server.getResources.ts +++ b/src/server.getResources.ts @@ -1,15 +1,24 @@ import { readFile } from 'node:fs/promises'; -import { isAbsolute, normalize, resolve } from 'node:path'; +import { isAbsolute, normalize, resolve, sep } from 'node:path'; import { getOptions } from './options.context'; import { DEFAULT_OPTIONS } from './options.defaults'; import { memo } from './server.caching'; import { normalizeString } from './server.search'; import { isUrl } from './server.helpers'; +import { log, formatUnknownError } from './logger'; + +interface ProcessedDoc { + content: string; + path: string | undefined; + resolvedPath: string | undefined; + isSuccess: boolean; +} /** * Read a local file and return its contents as a string * - * @param filePath + * @param filePath - Path to the file to be read. + * @returns The file contents as a string. */ const readLocalFileFunction = async (filePath: string) => await readFile(filePath, 'utf-8'); @@ -25,6 +34,7 @@ readLocalFileFunction.memo = memo(readLocalFileFunction, DEFAULT_OPTIONS.resourc * * @param url - URL to fetch * @param options - Options + * @returns The fetched content as a string. */ const fetchUrlFunction = async (url: string, options = getOptions()) => { const controller = new AbortController(); @@ -60,8 +70,32 @@ fetchUrlFunction.memo = memo(fetchUrlFunction, DEFAULT_OPTIONS.resourceMemoOptio * * @param path - Path to resolve. If it's relative, it will be resolved against the base directory.' * @param options - Options + * @returns Resolved file or URL path. + * + * @throws {Error} - Throws an error if the resolved path is invalid or outside the allowed base directory. */ const resolveLocalPathFunction = (path: string, options = getOptions()) => { + const documentationPrefix = options.docsPathSlug; + + // Safety check: Ensure the path is within the allowed directory + const confirmThenReturnResolvedBase = (base: string, resolved: string) => { + const normalizedBase = normalize(base); + const refinedBase = normalizedBase.endsWith(sep) ? normalizedBase : `${normalizedBase}${sep}`; + + if (!resolved.startsWith(refinedBase) && resolved !== normalizedBase) { + throw new Error(`Access denied: path ${path} is outside of allowed directory ${base}`); + } + + return resolved; + }; + + if (path.startsWith(documentationPrefix)) { + const base = options.docsPath; + const resolved = resolve(base, path.slice(documentationPrefix.length)); + + return confirmThenReturnResolvedBase(base, resolved); + } + if (isUrl(path)) { return path; } @@ -69,25 +103,51 @@ const resolveLocalPathFunction = (path: string, options = getOptions()) => { const base = options.contextPath; const resolved = isAbsolute(path) ? normalize(path) : resolve(base, path); - // Safety check: ensure the resolved path actually starts with the base directory - if (!resolved.startsWith(normalize(base))) { - throw new Error(`Access denied: path ${path} is outside of base directory ${base}`); + return confirmThenReturnResolvedBase(base, resolved); +}; + +/** + * Mock a given path or URL. Used for testing with fixture servers. + * + * @param pathOrUrl - Input path or URL to be resolved. + * @param options - Options + * @returns Resolves to the finalized URL or path as a memoized fetchable resource. + * + * @throws {Error} Throws an error if the given path cannot be resolved in the specified mode and is neither a valid URL nor fetchable. + */ +const mockPathOrUrlFunction = async (pathOrUrl: string, options = getOptions()) => { + const documentationPrefix = options.docsPathSlug; + const fixtureUrl = options.modeOptions?.test?.baseUrl; + let updatedPathOrUrl = pathOrUrl.startsWith(documentationPrefix) ? pathOrUrl : resolveLocalPathFunction(pathOrUrl); + + if (fixtureUrl) { + updatedPathOrUrl = `${fixtureUrl}${updatedPathOrUrl.startsWith('/') ? updatedPathOrUrl : `/${updatedPathOrUrl}`}`; + } else if (!isUrl(updatedPathOrUrl)) { + throw new Error(`Access denied: path ${updatedPathOrUrl} cannot be accessed in ${options.mode} mode`); } - return resolved; + // In test mode, everything is treated as a fetchable resource to allow mocking + return fetchUrlFunction.memo(updatedPathOrUrl); }; /** * Load a file from disk or `URL`, depending on the input type. * * @param pathOrUrl - Path or URL to load. If it's a URL, it will be fetched with `timeout` and `error` handling. + * @param options - Options + * @returns Resolves to an object containing the loaded content and the resolved path. */ -const loadFileFetch = async (pathOrUrl: string) => { - const isUrlStr = isUrl(pathOrUrl); - const updatedPathOrUrl = (isUrlStr && pathOrUrl) || resolveLocalPathFunction(pathOrUrl); +const loadFileFetch = async (pathOrUrl: string, options = getOptions()) => { + if (options.mode === 'test') { + const mockContent = await mockPathOrUrlFunction(pathOrUrl); + + return { content: mockContent, resolvedPath: pathOrUrl }; + } + + const updatedPathOrUrl = resolveLocalPathFunction(pathOrUrl); let content; - if (isUrlStr) { + if (isUrl(updatedPathOrUrl)) { content = await fetchUrlFunction.memo(updatedPathOrUrl); } else { content = await readLocalFileFunction.memo(updatedPathOrUrl); @@ -101,6 +161,7 @@ const loadFileFetch = async (pathOrUrl: string) => { * * @param queue - List of paths or URLs to load * @param limit - Optional limit on the number of concurrent promises. Defaults to 5. + * @returns An array of `PromiseSettledResult` objects, one for each input path or URL. */ const promiseQueue = async (queue: string[], limit = 5) => { const results = []; @@ -120,7 +181,9 @@ const promiseQueue = async (queue: string[], limit = 5) => { if (activeCount >= limit) { // Silent fail if one promise fails to load, but keep processing the rest. - await Promise.race(slidingQueue).catch(() => {}); + await Promise.race(slidingQueue).catch((reason: unknown) => { + log.debug(`Failed to load promise from queue: ${formatUnknownError(reason)}`); + }); } } @@ -133,11 +196,16 @@ const promiseQueue = async (queue: string[], limit = 5) => { * @note Remember to limit the number of docs to load to avoid OOM. * @param inputs - List of paths or URLs to load * @param options - Optional options + * @returns An array of loaded docs with content, path, resolvedPath, and isSuccess properties: + * - `content` is the loaded content string. + * - `path` is the original input path or URL. + * - `resolvedPath` is the resolved path after normalization. + * - `isSuccess` is true if the doc was successfully loaded, false otherwise. */ const processDocsFunction = async ( inputs: string[], options = getOptions() -) => { +): Promise => { const uniqueInputs = new Map( inputs.map(input => [normalizeString.memo(input), input.trim()]) ); @@ -176,4 +244,17 @@ const processDocsFunction = async ( return docs; }; -export { fetchUrlFunction, loadFileFetch, processDocsFunction, promiseQueue, readLocalFileFunction, resolveLocalPathFunction }; +/** + * Memoized version of processDocsFunction. Use default memo options. + */ +processDocsFunction.memo = memo(processDocsFunction, DEFAULT_OPTIONS.toolMemoOptions.usePatternFlyDocs); + +export { + fetchUrlFunction, + loadFileFetch, + processDocsFunction, + promiseQueue, + readLocalFileFunction, + resolveLocalPathFunction, + type ProcessedDoc +}; diff --git a/tests/__snapshots__/httpTransport.test.ts.snap b/tests/__snapshots__/httpTransport.test.ts.snap index 43f1b526..33ef65e8 100644 --- a/tests/__snapshots__/httpTransport.test.ts.snap +++ b/tests/__snapshots__/httpTransport.test.ts.snap @@ -17,113 +17,21 @@ exports[`Builtin resources, HTTP transport should expose expected resources and exports[`Builtin tools, HTTP transport should concatenate headers and separator with two local files 1`] = ` "# Documentation from documentation/guidelines/README.md -# PatternFly Guidelines - -Core development rules for AI coders building PatternFly React applications. - -## Related Files - -- [**Component Rules**](./component-architecture.md) - Component structure requirements -- [**Styling Rules**](./styling-standards.md) - CSS and styling requirements -- [**Layout Rules**](../components/layout/README.md) - Page structure requirements - -## Essential Rules - -### Version Requirements - -- ✅ **ALWAYS use PatternFly v6** - Use \`pf-v6-\` prefixed classes only -- ❌ **NEVER use legacy versions** - No \`pf-v5-\`, \`pf-v4-\`, or \`pf-c-\` classes -- ✅ **Match component and CSS versions** - Ensure compatibility - -### Component Usage Rules - -- ✅ **Use PatternFly components first** - Before creating custom solutions -- ✅ **Compose components** - Build complex UIs by combining PatternFly components -- ❌ **Don't override component internals** - Use provided props and APIs - -### Tokenss -- ✅ **ALWAYS use PatternFly tokens** - Use \`pf-t-\` prefixed classes over \`pf-v6-\` classes (e.g., \`var(--pf-t--global--spacer--sm)\` not \`var(--pf-v6-global--spacer--sm)\`) - -### Text Components (v6+) -\`\`\`jsx -// ✅ Correct -import { Content } from '@patternfly/react-core'; -Title - -// ❌ Wrong - Don't use old Text components -Title -\`\`\` - -### Icon Usage -\`\`\`jsx -// ✅ Correct - Wrap with Icon component -import { Icon } from '@patternfly/react-core'; -import { UserIcon } from '@patternfly/react-icons'; - -\`\`\` - -### Styling Rules - -- ✅ **Use PatternFly utilities** - Before writing custom CSS -- ✅ **Use semantic design tokens** for custom CSS (e.g., \`var(--pf-t--global--text--color--regular)\`), not base tokens with numbers (e.g., \`--pf-t--global--text--color--100\`) or hardcoded values -- ❌ **Don't mix PatternFly versions** - Stick to v6 throughout - -### Documentation Requirements - -1. **Check [PatternFly.org](https://www.patternfly.org/) first** - Primary source for APIs -2. **Check the [PatternFly React GitHub repository](https://github.com/patternfly/patternfly-react)** for the latest source code, examples, and release notes -3. **Use "View Code" sections** - Copy working examples -4. **Reference version-specific docs** - Match your project's PatternFly version -5. **Provide context to AI** - Share links and code snippets when asking for help - -> For the most up-to-date documentation, use both the official docs and the source repositories. When using AI tools, encourage them to leverage context7 to fetch the latest documentation from these sources. - -### Accessibility Requirements - -- ✅ **WCAG 2.1 AA compliance** - All components must meet standards -- ✅ **Proper ARIA labels** - Use semantic markup and labels -- ✅ **Keyboard navigation** - Ensure full keyboard accessibility -- ✅ **Focus management** - Logical focus order and visible indicators - -## Quality Checklist - -- [ ] Uses PatternFly v6 classes only -- [ ] Components render correctly across browsers -- [ ] Responsive on mobile and desktop -- [ ] Keyboard navigation works -- [ ] Screen readers can access content -- [ ] No console errors or warnings -- [ ] Performance is acceptable +# PatternFly Development Rules + This is a generated offline fixture used by the MCP external URLs test. -## When Issues Occur + Essential rules and guidelines working with PatternFly applications. -1. **Check [PatternFly.org](https://www.patternfly.org/)** - Verify component API -2. **Inspect elements** - Use browser dev tools for PatternFly classes -3. **Search [GitHub issues](https://github.com/patternfly/patternfly-react/issues)** - Look for similar problems -4. **Provide context** - Share code snippets and error messages + ## Quick Navigation -See [Common Issues](../troubleshooting/common-issues.md) for specific problems. + ### 🚀 Setup & Environment + - **Setup Rules** - Project initialization requirements + - **Quick Start** - Essential setup steps + - **Environment Rules** - Development configuration --- -# Documentation from documentation/components/README.md - -# PatternFly React Components - -You can find documentation on PatternFly's components at [PatternFly All components documentation](https://www.patternfly.org/components/all-components) - -## Specific info on Components - -- [AboutModal](https://www.patternfly.org/components/about-modal) -- [Accordion](https://raw.githubusercontent.com/patternfly/patternfly-org/refs/heads/main/packages/documentation-site/patternfly-docs/content/components/accordion/accordion.md) -- [ActionList](https://www.patternfly.org/components/action-list) -- [Alert](https://www.patternfly.org/components/alert) -- [ApplicationLauncher](https://www.patternfly.org/components/application-launcher) -" -`; - -exports[`Builtin tools, HTTP transport should concatenate headers and separator with two remote files 1`] = ` -"# Documentation from https://www.patternfly.org/notARealPath/README.md +# Documentation from documentation:components/README.md # PatternFly Development Rules This is a generated offline fixture used by the MCP external URLs test. @@ -135,7 +43,15 @@ exports[`Builtin tools, HTTP transport should concatenate headers and separator ### 🚀 Setup & Environment - **Setup Rules** - Project initialization requirements - **Quick Start** - Essential setup steps - - **Environment Rules** - Development configuration + - **Environment Rules** - Development configuration" +`; + +exports[`Builtin tools, HTTP transport should concatenate headers and separator with two remote files 1`] = ` +"# Documentation from https://www.patternfly.org/notARealPath/ChartLegend.md + +# Test Document + +This is a test document for mocking remote HTTP requests. --- @@ -158,7 +74,7 @@ exports[`Builtin tools, HTTP transport should expose expected tools and stable s exports[`Builtin tools, HTTP transport should initialize MCP session over HTTP 1`] = ` { - "baseUrl": "http://127.0.0.1:5001", + "baseUrl": "http://127.0.0.1:8000", "name": "@patternfly/patternfly-mcp", "version": "2024-11-05", } diff --git a/tests/__snapshots__/stdioTransport.test.ts.snap b/tests/__snapshots__/stdioTransport.test.ts.snap index c291f5fc..eb170aec 100644 --- a/tests/__snapshots__/stdioTransport.test.ts.snap +++ b/tests/__snapshots__/stdioTransport.test.ts.snap @@ -17,109 +17,33 @@ exports[`Builtin resources, STDIO should expose expected resources and templates exports[`Builtin tools, STDIO should concatenate headers and separator with two local files 1`] = ` "# Documentation from documentation/guidelines/README.md -# PatternFly Guidelines - -Core development rules for AI coders building PatternFly React applications. - -## Related Files - -- [**Component Rules**](./component-architecture.md) - Component structure requirements -- [**Styling Rules**](./styling-standards.md) - CSS and styling requirements -- [**Layout Rules**](../components/layout/README.md) - Page structure requirements - -## Essential Rules - -### Version Requirements - -- ✅ **ALWAYS use PatternFly v6** - Use \`pf-v6-\` prefixed classes only -- ❌ **NEVER use legacy versions** - No \`pf-v5-\`, \`pf-v4-\`, or \`pf-c-\` classes -- ✅ **Match component and CSS versions** - Ensure compatibility - -### Component Usage Rules - -- ✅ **Use PatternFly components first** - Before creating custom solutions -- ✅ **Compose components** - Build complex UIs by combining PatternFly components -- ❌ **Don't override component internals** - Use provided props and APIs - -### Tokenss -- ✅ **ALWAYS use PatternFly tokens** - Use \`pf-t-\` prefixed classes over \`pf-v6-\` classes (e.g., \`var(--pf-t--global--spacer--sm)\` not \`var(--pf-v6-global--spacer--sm)\`) - -### Text Components (v6+) -\`\`\`jsx -// ✅ Correct -import { Content } from '@patternfly/react-core'; -Title - -// ❌ Wrong - Don't use old Text components -Title -\`\`\` - -### Icon Usage -\`\`\`jsx -// ✅ Correct - Wrap with Icon component -import { Icon } from '@patternfly/react-core'; -import { UserIcon } from '@patternfly/react-icons'; - -\`\`\` - -### Styling Rules - -- ✅ **Use PatternFly utilities** - Before writing custom CSS -- ✅ **Use semantic design tokens** for custom CSS (e.g., \`var(--pf-t--global--text--color--regular)\`), not base tokens with numbers (e.g., \`--pf-t--global--text--color--100\`) or hardcoded values -- ❌ **Don't mix PatternFly versions** - Stick to v6 throughout - -### Documentation Requirements - -1. **Check [PatternFly.org](https://www.patternfly.org/) first** - Primary source for APIs -2. **Check the [PatternFly React GitHub repository](https://github.com/patternfly/patternfly-react)** for the latest source code, examples, and release notes -3. **Use "View Code" sections** - Copy working examples -4. **Reference version-specific docs** - Match your project's PatternFly version -5. **Provide context to AI** - Share links and code snippets when asking for help - -> For the most up-to-date documentation, use both the official docs and the source repositories. When using AI tools, encourage them to leverage context7 to fetch the latest documentation from these sources. - -### Accessibility Requirements - -- ✅ **WCAG 2.1 AA compliance** - All components must meet standards -- ✅ **Proper ARIA labels** - Use semantic markup and labels -- ✅ **Keyboard navigation** - Ensure full keyboard accessibility -- ✅ **Focus management** - Logical focus order and visible indicators - -## Quality Checklist - -- [ ] Uses PatternFly v6 classes only -- [ ] Components render correctly across browsers -- [ ] Responsive on mobile and desktop -- [ ] Keyboard navigation works -- [ ] Screen readers can access content -- [ ] No console errors or warnings -- [ ] Performance is acceptable +# PatternFly Development Rules + This is a generated offline fixture used by the MCP external URLs test. -## When Issues Occur + Essential rules and guidelines working with PatternFly applications. -1. **Check [PatternFly.org](https://www.patternfly.org/)** - Verify component API -2. **Inspect elements** - Use browser dev tools for PatternFly classes -3. **Search [GitHub issues](https://github.com/patternfly/patternfly-react/issues)** - Look for similar problems -4. **Provide context** - Share code snippets and error messages + ## Quick Navigation -See [Common Issues](../troubleshooting/common-issues.md) for specific problems. + ### 🚀 Setup & Environment + - **Setup Rules** - Project initialization requirements + - **Quick Start** - Essential setup steps + - **Environment Rules** - Development configuration --- -# Documentation from documentation/components/README.md +# Documentation from documentation:components/README.md -# PatternFly React Components +# PatternFly Development Rules + This is a generated offline fixture used by the MCP external URLs test. -You can find documentation on PatternFly's components at [PatternFly All components documentation](https://www.patternfly.org/components/all-components) + Essential rules and guidelines working with PatternFly applications. -## Specific info on Components + ## Quick Navigation -- [AboutModal](https://www.patternfly.org/components/about-modal) -- [Accordion](https://raw.githubusercontent.com/patternfly/patternfly-org/refs/heads/main/packages/documentation-site/patternfly-docs/content/components/accordion/accordion.md) -- [ActionList](https://www.patternfly.org/components/action-list) -- [Alert](https://www.patternfly.org/components/alert) -- [ApplicationLauncher](https://www.patternfly.org/components/application-launcher) -" + ### 🚀 Setup & Environment + - **Setup Rules** - Project initialization requirements + - **Quick Start** - Essential setup steps + - **Environment Rules** - Development configuration" `; exports[`Builtin tools, STDIO should concatenate headers and separator with two remote files 1`] = ` diff --git a/tests/httpTransport.test.ts b/tests/httpTransport.test.ts index 74809a74..8badc925 100644 --- a/tests/httpTransport.test.ts +++ b/tests/httpTransport.test.ts @@ -36,11 +36,14 @@ describe('Builtin tools, HTTP transport', () => { headers: { 'Content-Type': 'text/markdown; charset=utf-8' }, body: '# Test Document\n\nThis is a test document for mocking remote HTTP requests.' } - ], - excludePorts: [5001] + ] }); - CLIENT = await startServer({ http: { port: 5001 }, logging: { level: 'debug', protocol: true } }); + CLIENT = await startServer({ + isHttp: true, + modeOptions: { test: { baseUrl: FETCH_MOCK?.fixture?.baseUrl } }, + logging: { level: 'debug', protocol: true } + }); }); afterAll(async () => { @@ -85,7 +88,7 @@ describe('Builtin tools, HTTP transport', () => { arguments: { urlList: [ 'documentation/guidelines/README.md', - 'documentation/components/README.md' + 'documentation:components/README.md' ] } } @@ -108,7 +111,7 @@ describe('Builtin tools, HTTP transport', () => { name: 'usePatternFlyDocs', arguments: { urlList: [ - 'https://www.patternfly.org/notARealPath/README.md', + 'https://www.patternfly.org/notARealPath/ChartLegend.md', 'https://www.patternfly.org/notARealPath/AboutModal.md' ] } @@ -153,11 +156,14 @@ describe('Builtin resources, HTTP transport', () => { headers: { 'Content-Type': 'text/markdown; charset=utf-8' }, body: '# Test Document\n\nThis is a test document for mocking remote HTTP requests.' } - ], - excludePorts: [5002] + ] }); - CLIENT = await startServer({ http: { port: 5002 }, logging: { level: 'debug', protocol: true } }); + CLIENT = await startServer({ + isHttp: true, + modeOptions: { test: { baseUrl: FETCH_MOCK?.fixture?.baseUrl } }, + logging: { level: 'debug', protocol: true } + }); }); afterAll(async () => { diff --git a/tests/stdioTransport.test.ts b/tests/stdioTransport.test.ts index c96459c5..f6c98d26 100644 --- a/tests/stdioTransport.test.ts +++ b/tests/stdioTransport.test.ts @@ -47,7 +47,12 @@ describe('Builtin tools, STDIO', () => { }); URL_MOCK = `${FETCH_MOCK?.fixture?.baseUrl}/`; - CLIENT = await startServer(); + CLIENT = await startServer({ + args: [ + '--mode-test-url', + FETCH_MOCK?.fixture?.baseUrl + ] + }); }); afterAll(async () => { @@ -81,7 +86,7 @@ describe('Builtin tools, STDIO', () => { arguments: { urlList: [ 'documentation/guidelines/README.md', - 'documentation/components/README.md' + 'documentation:components/README.md' ] } } @@ -152,7 +157,12 @@ describe('Builtin resources, STDIO', () => { ] }); - CLIENT = await startServer(); + CLIENT = await startServer({ + args: [ + '--mode-test-url', + FETCH_MOCK?.fixture?.baseUrl + ] + }); }); afterAll(async () => { @@ -199,6 +209,17 @@ describe('Builtin resources, STDIO', () => { expect(content.text).toContain('PatternFly Documentation Index'); }); + it('should read a doc through a template', async () => { + const response = await CLIENT.send({ + method: 'resources/read', + params: { uri: 'patternfly://docs/Button' } + }); + const content = response?.result.contents[0]; + + expect(content.uri).toBe('patternfly://docs/Button'); + expect(content.text).toContain('This is a test document for mocking remote HTTP requests'); + }); + it('should read the patternfly-schemas-index', async () => { const response = await CLIENT.send({ method: 'resources/read', diff --git a/tests/utils/fetchMock.ts b/tests/utils/fetchMock.ts index 1e777d0d..165f4a1e 100644 --- a/tests/utils/fetchMock.ts +++ b/tests/utils/fetchMock.ts @@ -50,12 +50,13 @@ const startHttpFixture = ( // If route not found and we have regex routes, try to match them if (!route && regexRoutes.length > 0) { const pathname = url; + const pathnameNoSlash = url.startsWith('/') ? url.slice(1) : url; // Try to match against regex routes for (const regexRoute of regexRoutes) { if (regexRoute.url instanceof RegExp) { // Test regex against pathname - if (regexRoute.url.test(pathname) || regexRoute.url.test(`http://${address}${pathname}`)) { + if (regexRoute.url.test(pathname) || regexRoute.url.test(pathnameNoSlash) || regexRoute.url.test(`http://${address}${pathname}`)) { // Register this route dynamically route = { status: regexRoute.status || 200, @@ -72,7 +73,7 @@ const startHttpFixture = ( const pattern = regexRoute.url.replace(/\*/g, '.*'); const regex = new RegExp(`^${pattern}$`); - if (regex.test(pathname) || regex.test(`http://${address}${pathname}`)) { + if (regex.test(pathname) || regex.test(pathnameNoSlash) || regex.test(`http://${address}${pathname}`)) { route = { status: regexRoute.status || 200, headers: regexRoute.headers || { 'Content-Type': 'text/plain; charset=utf-8' }, @@ -288,8 +289,9 @@ export const setupFetchMock = async (options: FetchMockSetup = {}): Promise url.includes(`:${port}`)); + const isDocSlug = url.startsWith('documentation:'); - // Only intercept remote HTTP/HTTPS URLs that match our routes - if (!shouldExclude && (url.startsWith('http://') || url.startsWith('https://'))) { + // Only intercept remote HTTP/HTTPS URLs OR documentation protocol that match our routes + if (!shouldExclude && (url.startsWith('http://') || url.startsWith('https://') || isDocSlug)) { const matchedRoute = matchRoute(url); if (matchedRoute) { @@ -317,8 +320,8 @@ export const setupFetchMock = async (options: FetchMockSetup = {}): Promise & { level?: LoggingLevel }; toolModules?: PfMcpOptions['toolModules']; + modeOptions?: PfMcpOptions['modeOptions']; }; export type StartHttpServerSettings = PfMcpSettings; @@ -117,7 +118,7 @@ export const startServer = async ( httpClientUrl = new URL(baseUrl); } catch (error) { - throw new Error(`Failed to construct base URL: ${error}, host: ${host}, port: ${port}`); + throw new Error(`Failed to construct base URL: ${error}, host: ${host}, port: ${port}, httpClientPort: ${httpClientPort}`); } // Create MCP SDK client and transport diff --git a/tests/utils/stdioTransportClient.ts b/tests/utils/stdioTransportClient.ts index 30a5f7c3..fdfa795e 100644 --- a/tests/utils/stdioTransportClient.ts +++ b/tests/utils/stdioTransportClient.ts @@ -57,7 +57,7 @@ export const startServer = async ({ // Set stderr to 'pipe' so we can handle server logs separately from JSON-RPC messages const transport = new StdioClientTransport({ command, - args: [serverPath, ...args], + args: [serverPath, '--mode', 'test', ...args], env: { ...process.env, ...env } as any, stderr: 'pipe' // Pipe stderr so server logs don't interfere with JSON-RPC on stdout }); From ecd2d8aae7969b0f40a9f96c107f8af7bd61364c Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 5 Feb 2026 13:27:22 -0500 Subject: [PATCH 2/3] fix: review updates --- CONTRIBUTING.md | 8 ++++ docs/development.md | 78 ++++++++++++++++++++++++-------------- src/index.ts | 8 ++-- src/options.defaults.ts | 1 - src/server.getResources.ts | 8 ++-- 5 files changed, 68 insertions(+), 35 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 75adebfb..6bd374eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -127,6 +127,14 @@ Unit tests are located in the `__tests__` directory. E2E tests are located in the root `./tests` directory. +Contributors can run the MCP server in a specialized `test` mode against mock resources. + +```bash +npm run test:integration +``` + +This mode leverages the `--mode test` and `--mode-test-url` flags to redirect resource lookups to a fixture server instead of live or local resources. + ## AI agent ### User Section diff --git a/docs/development.md b/docs/development.md index 0041fcfb..f76772be 100644 --- a/docs/development.md +++ b/docs/development.md @@ -12,25 +12,28 @@ Complete guide to using the PatternFly MCP Server for development including CLI ### Available options -| Flag | Description | Default | -|:--------------------------------------|:------------------------------------------------------|:---------------------| -| `--http` | Enable HTTP transport mode | `false` (stdio mode) | -| `--port ` | Port for HTTP transport | `8080` | -| `--host ` | Host to bind to | `127.0.0.1` | -| `--allowed-origins ` | Comma-separated list of allowed CORS origins | `none` | -| `--allowed-hosts ` | Comma-separated list of allowed host headers | `none` | -| `--tool ` | Path to external Tool Plugin (repeatable) | `none` | -| `--plugin-isolation ` | Isolation preset for external tools-as-plugins | `strict` | -| `--log-stderr` | Enable terminal logging | `false` | -| `--log-protocol` | Forward logs to MCP clients | `false` | -| `--log-level ` | Set log level (`debug`, `info`, `warn`, `error`) | `info` | -| `--verbose` | Shortcut for `--log-level debug` | `false` | +| Flag | Description | Default | +|:--------------------------------------|:-------------------------------------------------|:---------------------| +| `--http` | Enable HTTP transport mode | `false` (stdio mode) | +| `--port ` | Port for HTTP transport | `8080` | +| `--host ` | Host to bind to | `127.0.0.1` | +| `--allowed-origins ` | Comma-separated list of allowed CORS origins | `none` | +| `--allowed-hosts ` | Comma-separated list of allowed host headers | `none` | +| `--tool ` | Path to external Tool Plugin (repeatable) | `none` | +| `--plugin-isolation ` | Isolation preset for external tools-as-plugins | `strict` | +| `--log-stderr` | Enable terminal logging | `false` | +| `--log-protocol` | Forward logs to MCP clients | `false` | +| `--log-level ` | Set log level (`debug`, `info`, `warn`, `error`) | `info` | +| `--mode ` | Operational mode (`cli`, `programmatic`, `test`) | `cli` | +| `--mode-test-url ` | Base URL for fixture/mock servers in `test` mode | `none` | +| `--verbose` | Shortcut for `--log-level debug` | `false` | #### Notes - **HTTP transport mode** - By default, the server uses `stdio`. Use the `--http` flag to enable HTTP transport. - **Logging** - The server uses a `diagnostics_channel`-based logger that keeps STDIO stdout pure by default. - **Programmatic API** - The server can also be used programmatically with options. See [Programmatic Usage](#programmatic-usage) for more details. -- **Tool Plugins** - The server can load external tool plugins at startup. See [Tool Plugins](#tool-plugins) for more details. +- **Tool Plugins** - The server can load external tool plugins at startup. See [Tool Plugins](#tool-plugins) for more details. +- **Test Mode** - When `--mode test` is used, the server redirects resource requests to the URL provided by `--mode-test-url`, enabling E2E testing without local filesystem access. ### Basic use scenarios @@ -53,6 +56,10 @@ npx @patternfly/patternfly-mcp --http --port 3000 --allowed-origins "https://app ```bash npx @patternfly/patternfly-mcp --tool ./first-tool.js --tool ./second-tool.ts ``` +**Testing with a fixture server**: +```bash +npx @patternfly/patternfly-mcp --mode test --mode-test-url "http://localhost:3000" +``` ### Testing with MCP Inspector @@ -81,20 +88,21 @@ npx @modelcontextprotocol/inspector-cli \ The `start()` function accepts an optional `PfMcpOptions` object for programmatic configuration. Use these options to customize behavior, transport, and logging for embedded instances. -| Option | Type | Description | Default | -|:----------------------|:-----------------------------------------|:----------------------------------------------------------------------|:-------------------| -| `toolModules` | `ToolModule \| ToolModule[]` | Array of tool modules or paths to external tool plugins to be loaded. | `[]` | -| `isHttp` | `boolean` | Enable HTTP transport mode. | `false` | -| `http.port` | `number` | Port for HTTP transport. | `8080` | -| `http.host` | `string` | Host to bind to. | `127.0.0.1` | -| `http.allowedOrigins` | `string[]` | List of allowed CORS origins. | `[]` | -| `http.allowedHosts` | `string[]` | List of allowed host headers. | `[]` | -| `pluginIsolation` | `'none' \| 'strict'` | Isolation preset for external tools-as-plugins. | `'strict'` | -| `logging.level` | `'debug' \| 'info' \| 'warn' \| 'error'` | Set the logging level. | `'info'` | -| `logging.stderr` | `boolean` | Enable terminal logging to stderr. | `false` | -| `logging.protocol` | `boolean` | Forward logs to MCP clients. | `false` | -| `mode` | `'cli' \| 'programmatic' \| 'test'` | Specifies the operation mode. | `'programmatic'` | -| `docsPath` | `string` | Path to the documentation directory. | (Internal default) | +| Option | Type | Description | Default | +|:---------------------------|:-----------------------------------------|:----------------------------------------------------------------------|:-------------------| +| `toolModules` | `ToolModule \| ToolModule[]` | Array of tool modules or paths to external tool plugins to be loaded. | `[]` | +| `isHttp` | `boolean` | Enable HTTP transport mode. | `false` | +| `http.port` | `number` | Port for HTTP transport. | `8080` | +| `http.host` | `string` | Host to bind to. | `127.0.0.1` | +| `http.allowedOrigins` | `string[]` | List of allowed CORS origins. | `[]` | +| `http.allowedHosts` | `string[]` | List of allowed host headers. | `[]` | +| `pluginIsolation` | `'none' \| 'strict'` | Isolation preset for external tools-as-plugins. | `'strict'` | +| `logging.level` | `'debug' \| 'info' \| 'warn' \| 'error'` | Set the logging level. | `'info'` | +| `logging.stderr` | `boolean` | Enable terminal logging to stderr. | `false` | +| `logging.protocol` | `boolean` | Forward logs to MCP clients. | `false` | +| `mode` | `'cli' \| 'programmatic' \| 'test'` | Specifies the operation mode. | `'programmatic'` | +| `modeOptions.test.baseUrl` | `string` | Base URL for fixture/mock servers in `test` mode. | `undefined` | +| `docsPath` | `string` | Path to the documentation directory. | (Internal default) | #### Example usage @@ -116,6 +124,20 @@ const options: PfMcpOptions = { const server: PfMcpInstance = await start(options); ``` +**Example: Programmatic test mode** +```typescript +import { start, type PfMcpInstance } from '@patternfly/patternfly-mcp'; + +const server: PfMcpInstance = await start({ + mode: 'test', + modeOptions: { + test: { + baseUrl: 'http://my-fixture-server:3000' + } + } +}); +``` + ### Server instance The server instance exposes the following methods: diff --git a/src/index.ts b/src/index.ts index fc24c20a..9b4d9b1e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -156,13 +156,15 @@ const main = async ( let updatedAllowProcessExit = allowProcessExit ?? programmaticMode !== 'test'; let mergedOptions: ServerOptions; - // If allowed, exit the process on error + // If allowed, exit the process on error otherwise log then throw the error. const processExit = (message: string, error: unknown) => { console.error(message, error); if (updatedAllowProcessExit) { process.exit(1); } + + throw error; }; try { @@ -176,7 +178,6 @@ const main = async ( updatedAllowProcessExit = allowProcessExit ?? mergedOptions.mode !== 'test'; } catch (error) { processExit('Set options error, failed to start server:', error); - throw error; } try { @@ -188,8 +189,9 @@ const main = async ( await runServer.memo(mergedOptions, { allowProcessExit: updatedAllowProcessExit })); } catch (error) { processExit('Failed to start server:', error); - throw error; } + + return undefined as never; }; export { diff --git a/src/options.defaults.ts b/src/options.defaults.ts index e92bdaaa..d12f663c 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -348,7 +348,6 @@ const LOG_BASENAME = 'pf-mcp:log'; * Default PatternFly-specific options. */ const PATTERNFLY_OPTIONS: PatternFlyOptions = { - // availableVersions: ['v3', 'v4', 'v5', 'v6'], availableResourceVersions: ['v6'], default: { defaultVersion: 'v6', diff --git a/src/server.getResources.ts b/src/server.getResources.ts index d6469297..8ee751c4 100644 --- a/src/server.getResources.ts +++ b/src/server.getResources.ts @@ -78,7 +78,7 @@ const resolveLocalPathFunction = (path: string, options = getOptions()) => { const documentationPrefix = options.docsPathSlug; // Safety check: Ensure the path is within the allowed directory - const confirmThenReturnResolvedBase = (base: string, resolved: string) => { + const assertPathWithinBaseAndReturn = (base: string, resolved: string) => { const normalizedBase = normalize(base); const refinedBase = normalizedBase.endsWith(sep) ? normalizedBase : `${normalizedBase}${sep}`; @@ -89,13 +89,15 @@ const resolveLocalPathFunction = (path: string, options = getOptions()) => { return resolved; }; + // Paths starting with the documentation prefix are resolved relative to the documentation path if (path.startsWith(documentationPrefix)) { const base = options.docsPath; const resolved = resolve(base, path.slice(documentationPrefix.length)); - return confirmThenReturnResolvedBase(base, resolved); + return assertPathWithinBaseAndReturn(base, resolved); } + // URLs are returned as-is if (isUrl(path)) { return path; } @@ -103,7 +105,7 @@ const resolveLocalPathFunction = (path: string, options = getOptions()) => { const base = options.contextPath; const resolved = isAbsolute(path) ? normalize(path) : resolve(base, path); - return confirmThenReturnResolvedBase(base, resolved); + return assertPathWithinBaseAndReturn(base, resolved); }; /** From 23d87a5969b71d05da5c7c6cd711d3bfbf4eadc7 Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Thu, 5 Feb 2026 14:07:41 -0500 Subject: [PATCH 3/3] fix: review update --- src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index.ts b/src/index.ts index 9b4d9b1e..f0639779 100644 --- a/src/index.ts +++ b/src/index.ts @@ -191,6 +191,7 @@ const main = async ( processExit('Failed to start server:', error); } + // Unreachable, processExit exits or throws. Kept for type satisfaction. return undefined as never; };