diff --git a/eslint.config.js b/eslint.config.js index 600c872..0030238 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -52,6 +52,20 @@ export default [ '@typescript-eslint': tseslint.plugin }, rules: { + '@typescript-eslint/consistent-type-imports': [ + 2, + { + prefer: 'type-imports', + fixStyle: 'inline-type-imports', + disallowTypeAnnotations: false + } + ], + '@typescript-eslint/consistent-type-exports': [ + 2, + { + fixMixedExportsWithInlineTypeSpecifier: true + } + ], '@typescript-eslint/no-explicit-any': 1, '@typescript-eslint/explicit-function-return-type': 0, '@typescript-eslint/no-unused-vars': ['error', { diff --git a/src/__tests__/__snapshots__/options.test.ts.snap b/src/__tests__/__snapshots__/options.test.ts.snap index 98cdae1..3f817c6 100644 --- a/src/__tests__/__snapshots__/options.test.ts.snap +++ b/src/__tests__/__snapshots__/options.test.ts.snap @@ -8,9 +8,6 @@ exports[`parseCliOptions should attempt to parse args with --allowed-hosts 1`] = "localhost", "127.0.0.1", ], - "allowedOrigins": undefined, - "host": undefined, - "port": undefined, }, "isHttp": true, "logging": { @@ -27,13 +24,10 @@ exports[`parseCliOptions should attempt to parse args with --allowed-origins 1`] { "docsHost": false, "http": { - "allowedHosts": undefined, "allowedOrigins": [ "https://app.com", "https://admin.app.com", ], - "host": undefined, - "port": undefined, }, "isHttp": true, "logging": { @@ -49,7 +43,7 @@ exports[`parseCliOptions should attempt to parse args with --allowed-origins 1`] exports[`parseCliOptions should attempt to parse args with --docs-host flag 1`] = ` { "docsHost": true, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "info", @@ -65,10 +59,7 @@ exports[`parseCliOptions should attempt to parse args with --http and --host 1`] { "docsHost": false, "http": { - "allowedHosts": undefined, - "allowedOrigins": undefined, "host": "0.0.0.0", - "port": undefined, }, "isHttp": true, "logging": { @@ -85,10 +76,7 @@ exports[`parseCliOptions should attempt to parse args with --http and --port 1`] { "docsHost": false, "http": { - "allowedHosts": undefined, - "allowedOrigins": undefined, - "host": undefined, - "port": undefined, + "port": 6000, }, "isHttp": true, "logging": { @@ -101,15 +89,25 @@ exports[`parseCliOptions should attempt to parse args with --http and --port 1`] } `; -exports[`parseCliOptions should attempt to parse args with --http flag 1`] = ` +exports[`parseCliOptions should attempt to parse args with --http and invalid --port 1`] = ` { "docsHost": false, - "http": { - "allowedHosts": undefined, - "allowedOrigins": undefined, - "host": undefined, - "port": undefined, + "http": {}, + "isHttp": true, + "logging": { + "level": "info", + "logger": "@patternfly/patternfly-mcp", + "protocol": false, + "stderr": false, + "transport": "stdio", }, +} +`; + +exports[`parseCliOptions should attempt to parse args with --http flag 1`] = ` +{ + "docsHost": false, + "http": {}, "isHttp": true, "logging": { "level": "info", @@ -124,7 +122,7 @@ exports[`parseCliOptions should attempt to parse args with --http flag 1`] = ` exports[`parseCliOptions should attempt to parse args with --log-level flag 1`] = ` { "docsHost": false, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "warn", @@ -139,7 +137,7 @@ exports[`parseCliOptions should attempt to parse args with --log-level flag 1`] exports[`parseCliOptions should attempt to parse args with --log-stderr flag and --log-protocol flag 1`] = ` { "docsHost": false, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "info", @@ -154,7 +152,7 @@ exports[`parseCliOptions should attempt to parse args with --log-stderr flag and exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] = ` { "docsHost": false, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "debug", @@ -169,7 +167,7 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] = exports[`parseCliOptions should attempt to parse args with --verbose flag and --log-level flag 1`] = ` { "docsHost": false, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "debug", @@ -184,7 +182,7 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag and -- exports[`parseCliOptions should attempt to parse args with other arguments 1`] = ` { "docsHost": false, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "info", @@ -199,7 +197,7 @@ exports[`parseCliOptions should attempt to parse args with other arguments 1`] = exports[`parseCliOptions should attempt to parse args without --docs-host flag 1`] = ` { "docsHost": false, - "http": undefined, + "http": {}, "isHttp": false, "logging": { "level": "info", diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 184f2ef..8c18b89 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -35,13 +35,13 @@ describe('main', () => { mockParseCliOptions.mockImplementation(() => { callOrder.push('parse'); - return { docsHost: false, logging: defaultLogging } as unknown as CliOptions; + return { docsHost: false, logging: defaultLogging } as CliOptions; }); mockSetOptions.mockImplementation(options => { callOrder.push('set'); - return Object.freeze({ ...DEFAULT_OPTIONS, ...options }) as unknown as GlobalOptions; + return Object.freeze({ ...DEFAULT_OPTIONS, ...options }) as GlobalOptions; }); mockGetSessionOptions.mockReturnValue({ diff --git a/src/__tests__/options.test.ts b/src/__tests__/options.test.ts index fd7e240..b202056 100644 --- a/src/__tests__/options.test.ts +++ b/src/__tests__/options.test.ts @@ -42,7 +42,11 @@ describe('parseCliOptions', () => { }, { description: 'with --http and --port', - args: ['node', 'script.js', '--http', '--port', '8080'] + args: ['node', 'script.js', '--http', '--port', '6000'] + }, + { + description: 'with --http and invalid --port', + args: ['node', 'script.js', '--http', '--port', '0'] }, { description: 'with --http and --host', @@ -63,4 +67,16 @@ describe('parseCliOptions', () => { expect(result).toMatchSnapshot(); }); + + it('parses from a provided argv independent of process.argv', () => { + const customArgv = ['node', 'cli', '--http', '--port', '3101']; + const result = parseCliOptions(customArgv); + expect(result.http?.port).toBe(3101); + }); + + it('trims spaces in list flags', () => { + const argv = ['node', 'cli', '--http', '--allowed-hosts', ' localhost , 127.0.0.1 ']; + const result = parseCliOptions(argv); + expect(result.http?.allowedHosts).toEqual(['localhost', '127.0.0.1']); + }); }); diff --git a/src/index.ts b/src/index.ts index 8227bd4..c749a8b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,9 +1,5 @@ -import { parseCliOptions, type CliOptions, type DefaultOptions } from './options'; -import { - getSessionOptions, - setOptions, - runWithSession -} from './options.context'; +import { parseCliOptions, type CliOptions, type DefaultOptionsOverrides } from './options'; +import { getSessionOptions, setOptions, runWithSession } from './options.context'; import { runServer, type ServerInstance, @@ -24,7 +20,7 @@ import { * - `'programmatic'`: Functionality is invoked programmatically. Allows process exits. * - `'test'`: Functionality is being tested. Does NOT allow process exits. */ -type PfMcpOptions = Partial & { +type PfMcpOptions = DefaultOptionsOverrides & { mode?: 'cli' | 'programmatic' | 'test'; }; diff --git a/src/options.context.ts b/src/options.context.ts index b24b9dc..6739f0e 100644 --- a/src/options.context.ts +++ b/src/options.context.ts @@ -1,7 +1,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { randomUUID } from 'node:crypto'; -import { type Session, type GlobalOptions } from './options'; -import { DEFAULT_OPTIONS, LOG_BASENAME, type LoggingSession, type DefaultOptions } from './options.defaults'; +import { type AppSession, type GlobalOptions, type DefaultOptionsOverrides } from './options'; +import { DEFAULT_OPTIONS, LOG_BASENAME, type LoggingSession } from './options.defaults'; import { mergeObjects, freezeObject, isPlainObject } from './server.helpers'; /** @@ -10,14 +10,14 @@ import { mergeObjects, freezeObject, isPlainObject } from './server.helpers'; * The `sessionContext` allows sharing a common context without explicitly * passing it as a parameter. */ -const sessionContext = new AsyncLocalStorage(); +const sessionContext = new AsyncLocalStorage(); /** * Initialize and return session data. * - * @returns {Session} Immutable session with a session ID and channel name. + * @returns {AppSession} Immutable session with a session ID and channel name. */ -const initializeSession = (): Session => { +const initializeSession = (): AppSession => { const sessionId = (process.env.NODE_ENV === 'local' && '1234d567-1ce9-123d-1413-a1234e56c789') || randomUUID(); const channelName = `${LOG_BASENAME}:${sessionId}`; @@ -27,10 +27,10 @@ const initializeSession = (): Session => { /** * Set and return the current session options. * - * @param {Session} [session] - * @returns {Session} + * @param {AppSession} [session] + * @returns {AppSession} */ -const setSessionOptions = (session: Session = initializeSession()) => { +const setSessionOptions = (session: AppSession = initializeSession()) => { sessionContext.enterWith(session); return session; @@ -39,10 +39,10 @@ const setSessionOptions = (session: Session = initializeSession()) => { /** * Get the current session options or set a new session with defaults. */ -const getSessionOptions = (): Session => sessionContext.getStore() || setSessionOptions(); +const getSessionOptions = (): AppSession => sessionContext.getStore() || setSessionOptions(); const runWithSession = async ( - session: Session, + session: AppSession, callback: () => TReturn | Promise ) => { const frozen = freezeObject(structuredClone(session)); @@ -61,10 +61,10 @@ const optionsContext = new AsyncLocalStorage(); /** * Set and freeze cloned options in the current async context. * - * @param {Partial} [options] - Optional options to set in context. Merged with DEFAULT_OPTIONS. + * @param {DefaultOptionsOverrides} [options] - Optional overrides merged with DEFAULT_OPTIONS. * @returns {GlobalOptions} Cloned frozen default options object with session. */ -const setOptions = (options?: Partial): GlobalOptions => { +const setOptions = (options?: DefaultOptionsOverrides): GlobalOptions => { const base = mergeObjects(DEFAULT_OPTIONS, options, { allowNullValues: false, allowUndefinedValues: false }); const baseLogging = isPlainObject(base.logging) ? base.logging : DEFAULT_OPTIONS.logging; const merged: GlobalOptions = { @@ -102,7 +102,7 @@ const getOptions = (): GlobalOptions => optionsContext.getStore() || setOptions( /** * Get logging options from the current context. * - * @param {Session} [session] - Session options to use in context. + * @param {AppSession} [session] - Session options to use in context. * @returns {LoggingSession} Logging options from context. */ const getLoggerOptions = (session = getSessionOptions()): LoggingSession => { diff --git a/src/options.defaults.ts b/src/options.defaults.ts index 66007cf..9275388 100644 --- a/src/options.defaults.ts +++ b/src/options.defaults.ts @@ -35,7 +35,7 @@ interface DefaultOptions { contextPath: string; docsHost: boolean; docsPath: string; - http: HttpOptions | undefined; + http: HttpOptions; isHttp: boolean; llmsFilesPath: string; logging: TLogOptions; @@ -57,11 +57,22 @@ interface DefaultOptions { version: string; } +/** + * Overrides for default options. + */ +type DefaultOptionsOverrides = Partial< + Omit +> & { + http?: Partial; + logging?: Partial; +}; + /** * Logging options. * + * See `LOGGING_OPTIONS` for defaults. + * * @interface LoggingOptions - * @default { level: 'debug', logger: packageJson.name, stderr: false, protocol: false, transport: 'stdio' } * * @property level Logging level. * @property logger Logger name. Human-readable/configurable logger name used in MCP protocol messages. Isolated @@ -82,8 +93,9 @@ interface LoggingOptions { /** * HTTP server options. * + * See `HTTP_OPTIONS` for defaults. + * * @interface HttpOptions - * @default { port: 8080, host: '127.0.0.1', allowedOrigins: [], allowedHosts: [] } * * @property port Port number. * @property host Host name. @@ -288,6 +300,7 @@ export { LOG_BASENAME, DEFAULT_OPTIONS, type DefaultOptions, + type DefaultOptionsOverrides, type HttpOptions, type LoggingOptions, type LoggingSession diff --git a/src/options.ts b/src/options.ts index 54ee455..ebced89 100644 --- a/src/options.ts +++ b/src/options.ts @@ -1,10 +1,10 @@ -import { DEFAULT_OPTIONS, type DefaultOptions, type LoggingOptions, type HttpOptions } from './options.defaults'; +import { DEFAULT_OPTIONS, type DefaultOptions, type DefaultOptionsOverrides, type LoggingOptions, type HttpOptions } from './options.defaults'; import { type LogLevel, logSeverity } from './logger'; /** * Session defaults, not user-configurable */ -type Session = { +type AppSession = { readonly sessionId: string; readonly channelName: string }; @@ -19,25 +19,27 @@ type GlobalOptions = DefaultOptions; */ type CliOptions = { docsHost: boolean; - http: HttpOptions | undefined; + http?: Partial; isHttp: boolean; - logging: LoggingOptions; + logging: Partial; }; /** - * Get argument value from process.argv + * Get argument value from argv (defaults to `process.argv`). * * @param flag - CLI flag to search for - * @param defaultValue - Default arg value + * @param [options] - Options + * @param [options.defaultValue] - Default arg value + * @param [options.argv] - Command-line arguments to parse. Defaults to `process.argv`. */ -const getArgValue = (flag: string, defaultValue?: unknown) => { - const index = process.argv.indexOf(flag); +const getArgValue = (flag: string, { defaultValue, argv = process.argv }: { defaultValue?: unknown, argv?: string[] } = {}) => { + const index = argv.indexOf(flag); if (index === -1) { return defaultValue; } - const value = process.argv[index + 1]; + const value = argv[index + 1]; if (!value || value.startsWith('-')) { return defaultValue; @@ -61,17 +63,17 @@ const getArgValue = (flag: string, defaultValue?: unknown) => { * * 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`. + * - `--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 * - `--log-protocol`: Enables MCP protocol logging. Forward server logs to MCP clients (requires advertising `capabilities.logging`). * - `--http`: Indicates if the `--http` option is enabled. - * - `--port`: The port number specified via `--port`, or defaults to `3000` if not provided. - * - `--host`: The host name specified via `--host`, or defaults to `'127.0.0.1'` if not provided. + * - `--port`: The port number specified via `--port` + * - `--host`: The host name specified via `--host` * - `--allowed-origins`: List of allowed origins derived from the `--allowed-origins` parameter, split by commas, or undefined if not provided. * - `--allowed-hosts`: List of allowed hosts derived from the `--allowed-hosts` parameter, split by commas, or undefined if not provided. * - * @param argv - Command-line arguments to parse. Defaults to `process.argv`. + * @param [argv] - Command-line arguments to parse. Defaults to `process.argv`. * @returns Parsed command-line options. */ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { @@ -94,24 +96,41 @@ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { } const isHttp = argv.includes('--http'); - let http: HttpOptions | undefined; + const http: Partial = {}; if (isHttp) { - let port = getArgValue('--port'); - const host = getArgValue('--host'); - const allowedOrigins = (getArgValue('--allowed-origins') as string)?.split(',')?.filter((origin: string) => origin.trim()); - const allowedHosts = (getArgValue('--allowed-hosts') as string)?.split(',')?.filter((host: string) => host.trim()); + const rawPort = getArgValue('--port', { argv }); + const parsedPort = Number.parseInt(String(rawPort || ''), 10); + const host = getArgValue('--host', { argv }); - const isPortValid = (typeof port === 'number') && (port > 0 && port < 65536); + const allowedOrigins = (getArgValue('--allowed-origins', { argv }) as string) + ?.split(',') + ?.map((origin: string) => origin.trim()) + ?.filter(Boolean); - port = isPortValid ? port : undefined; + const allowedHosts = (getArgValue('--allowed-hosts', { argv }) as string) + ?.split(',') + ?.map((host: string) => host.trim()) + ?.filter(Boolean); - http = { - port, - host, - allowedHosts, - allowedOrigins - } as HttpOptions; + const isPortValid = Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort < 65536; + const port = isPortValid ? parsedPort : undefined; + + if (port !== undefined) { + http.port = port; + } + + if (typeof host === 'string') { + http.host = host; + } + + if (Array.isArray(allowedHosts) && allowedHosts.length) { + http.allowedHosts = allowedHosts; + } + + if (Array.isArray(allowedOrigins) && allowedOrigins.length) { + http.allowedOrigins = allowedOrigins; + } } return { docsHost, logging, isHttp, http }; @@ -120,10 +139,11 @@ const parseCliOptions = (argv: string[] = process.argv): CliOptions => { export { parseCliOptions, getArgValue, + type AppSession, type CliOptions, type DefaultOptions, + type DefaultOptionsOverrides, type GlobalOptions, type HttpOptions, - type LoggingOptions, - type Session + type LoggingOptions }; diff --git a/src/server.helpers.ts b/src/server.helpers.ts index 87ed8d6..d06526a 100644 --- a/src/server.helpers.ts +++ b/src/server.helpers.ts @@ -32,7 +32,8 @@ const isPlainObject = (obj: unknown): obj is Record => { * Prototype-pollution keys are ignored. * * @param baseObj - Base object to merge into - * @param sourceObj - Source object to merge from + * @param sourceObj - Source object to merge from. Source may be `undefined` or `any` value; `non‑plain objects` are ignored, and the + * base is returned cloned. * @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` @@ -40,7 +41,7 @@ const isPlainObject = (obj: unknown): obj is Record => { */ const mergeObjects = ( baseObj: TBase, - sourceObj?: Partial | null, + sourceObj?: unknown, { allowNullValues = true, allowUndefinedValues = false diff --git a/src/server.http.ts b/src/server.http.ts index 7b878d9..33e3608 100644 --- a/src/server.http.ts +++ b/src/server.http.ts @@ -1,10 +1,10 @@ -import { createServer, IncomingMessage, ServerResponse } from 'node:http'; -import { Socket } from 'node:net'; +import { createServer, type IncomingMessage, type ServerResponse } from 'node:http'; +import { type Socket } from 'node:net'; import { execSync } from 'node:child_process'; import { platform } from 'node:os'; import { randomUUID } from 'node:crypto'; import { StreamableHTTPServerTransport, type StreamableHTTPServerTransportOptions } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { portToPid } from 'pid-port'; import { getOptions } from './options.context'; import { log } from './logger'; @@ -95,7 +95,7 @@ const getProcessOnPort = async (port: number) => { /** * Create Streamable HTTP transport * - * @param {DefaultSession} [options] + * @param {GlobalOptions} [options] */ const createStreamableHttpTransport = (options = getOptions()) => { const { http } = options; @@ -149,7 +149,7 @@ type HttpServerHandle = { * Start the HTTP transport server * * @param {McpServer} mcpServer - * @param {DefaultSession} [options] + * @param {GlobalOptions} [options] * @returns Handle with close method for server lifecycle management */ const startHttpTransport = async (mcpServer: McpServer, options = getOptions()): Promise => {