Skip to content

Commit 727fc0a

Browse files
cdcabreracdcabrera
authored
fix: cli http port option, typings (#40)
* typings, internal clean up, annotations * options, corrected port not passing through options
1 parent 6a65e19 commit 727fc0a

10 files changed

Lines changed: 145 additions & 87 deletions

File tree

‎eslint.config.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,20 @@ export default [
5252
'@typescript-eslint': tseslint.plugin
5353
},
5454
rules: {
55+
'@typescript-eslint/consistent-type-imports': [
56+
2,
57+
{
58+
prefer: 'type-imports',
59+
fixStyle: 'inline-type-imports',
60+
disallowTypeAnnotations: false
61+
}
62+
],
63+
'@typescript-eslint/consistent-type-exports': [
64+
2,
65+
{
66+
fixMixedExportsWithInlineTypeSpecifier: true
67+
}
68+
],
5569
'@typescript-eslint/no-explicit-any': 1,
5670
'@typescript-eslint/explicit-function-return-type': 0,
5771
'@typescript-eslint/no-unused-vars': ['error', {

‎src/__tests__/__snapshots__/options.test.ts.snap‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ exports[`parseCliOptions should attempt to parse args with --allowed-hosts 1`] =
88
"localhost",
99
"127.0.0.1",
1010
],
11-
"allowedOrigins": undefined,
12-
"host": undefined,
13-
"port": undefined,
1411
},
1512
"isHttp": true,
1613
"logging": {
@@ -27,13 +24,10 @@ exports[`parseCliOptions should attempt to parse args with --allowed-origins 1`]
2724
{
2825
"docsHost": false,
2926
"http": {
30-
"allowedHosts": undefined,
3127
"allowedOrigins": [
3228
"https://app.com",
3329
"https://admin.app.com",
3430
],
35-
"host": undefined,
36-
"port": undefined,
3731
},
3832
"isHttp": true,
3933
"logging": {
@@ -49,7 +43,7 @@ exports[`parseCliOptions should attempt to parse args with --allowed-origins 1`]
4943
exports[`parseCliOptions should attempt to parse args with --docs-host flag 1`] = `
5044
{
5145
"docsHost": true,
52-
"http": undefined,
46+
"http": {},
5347
"isHttp": false,
5448
"logging": {
5549
"level": "info",
@@ -65,10 +59,7 @@ exports[`parseCliOptions should attempt to parse args with --http and --host 1`]
6559
{
6660
"docsHost": false,
6761
"http": {
68-
"allowedHosts": undefined,
69-
"allowedOrigins": undefined,
7062
"host": "0.0.0.0",
71-
"port": undefined,
7263
},
7364
"isHttp": true,
7465
"logging": {
@@ -85,10 +76,7 @@ exports[`parseCliOptions should attempt to parse args with --http and --port 1`]
8576
{
8677
"docsHost": false,
8778
"http": {
88-
"allowedHosts": undefined,
89-
"allowedOrigins": undefined,
90-
"host": undefined,
91-
"port": undefined,
79+
"port": 6000,
9280
},
9381
"isHttp": true,
9482
"logging": {
@@ -101,15 +89,25 @@ exports[`parseCliOptions should attempt to parse args with --http and --port 1`]
10189
}
10290
`;
10391

104-
exports[`parseCliOptions should attempt to parse args with --http flag 1`] = `
92+
exports[`parseCliOptions should attempt to parse args with --http and invalid --port 1`] = `
10593
{
10694
"docsHost": false,
107-
"http": {
108-
"allowedHosts": undefined,
109-
"allowedOrigins": undefined,
110-
"host": undefined,
111-
"port": undefined,
95+
"http": {},
96+
"isHttp": true,
97+
"logging": {
98+
"level": "info",
99+
"logger": "@patternfly/patternfly-mcp",
100+
"protocol": false,
101+
"stderr": false,
102+
"transport": "stdio",
112103
},
104+
}
105+
`;
106+
107+
exports[`parseCliOptions should attempt to parse args with --http flag 1`] = `
108+
{
109+
"docsHost": false,
110+
"http": {},
113111
"isHttp": true,
114112
"logging": {
115113
"level": "info",
@@ -124,7 +122,7 @@ exports[`parseCliOptions should attempt to parse args with --http flag 1`] = `
124122
exports[`parseCliOptions should attempt to parse args with --log-level flag 1`] = `
125123
{
126124
"docsHost": false,
127-
"http": undefined,
125+
"http": {},
128126
"isHttp": false,
129127
"logging": {
130128
"level": "warn",
@@ -139,7 +137,7 @@ exports[`parseCliOptions should attempt to parse args with --log-level flag 1`]
139137
exports[`parseCliOptions should attempt to parse args with --log-stderr flag and --log-protocol flag 1`] = `
140138
{
141139
"docsHost": false,
142-
"http": undefined,
140+
"http": {},
143141
"isHttp": false,
144142
"logging": {
145143
"level": "info",
@@ -154,7 +152,7 @@ exports[`parseCliOptions should attempt to parse args with --log-stderr flag and
154152
exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] = `
155153
{
156154
"docsHost": false,
157-
"http": undefined,
155+
"http": {},
158156
"isHttp": false,
159157
"logging": {
160158
"level": "debug",
@@ -169,7 +167,7 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag 1`] =
169167
exports[`parseCliOptions should attempt to parse args with --verbose flag and --log-level flag 1`] = `
170168
{
171169
"docsHost": false,
172-
"http": undefined,
170+
"http": {},
173171
"isHttp": false,
174172
"logging": {
175173
"level": "debug",
@@ -184,7 +182,7 @@ exports[`parseCliOptions should attempt to parse args with --verbose flag and --
184182
exports[`parseCliOptions should attempt to parse args with other arguments 1`] = `
185183
{
186184
"docsHost": false,
187-
"http": undefined,
185+
"http": {},
188186
"isHttp": false,
189187
"logging": {
190188
"level": "info",
@@ -199,7 +197,7 @@ exports[`parseCliOptions should attempt to parse args with other arguments 1`] =
199197
exports[`parseCliOptions should attempt to parse args without --docs-host flag 1`] = `
200198
{
201199
"docsHost": false,
202-
"http": undefined,
200+
"http": {},
203201
"isHttp": false,
204202
"logging": {
205203
"level": "info",

‎src/__tests__/index.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ describe('main', () => {
3535
mockParseCliOptions.mockImplementation(() => {
3636
callOrder.push('parse');
3737

38-
return { docsHost: false, logging: defaultLogging } as unknown as CliOptions;
38+
return { docsHost: false, logging: defaultLogging } as CliOptions;
3939
});
4040

4141
mockSetOptions.mockImplementation(options => {
4242
callOrder.push('set');
4343

44-
return Object.freeze({ ...DEFAULT_OPTIONS, ...options }) as unknown as GlobalOptions;
44+
return Object.freeze({ ...DEFAULT_OPTIONS, ...options }) as GlobalOptions;
4545
});
4646

4747
mockGetSessionOptions.mockReturnValue({

‎src/__tests__/options.test.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ describe('parseCliOptions', () => {
4242
},
4343
{
4444
description: 'with --http and --port',
45-
args: ['node', 'script.js', '--http', '--port', '8080']
45+
args: ['node', 'script.js', '--http', '--port', '6000']
46+
},
47+
{
48+
description: 'with --http and invalid --port',
49+
args: ['node', 'script.js', '--http', '--port', '0']
4650
},
4751
{
4852
description: 'with --http and --host',
@@ -63,4 +67,16 @@ describe('parseCliOptions', () => {
6367

6468
expect(result).toMatchSnapshot();
6569
});
70+
71+
it('parses from a provided argv independent of process.argv', () => {
72+
const customArgv = ['node', 'cli', '--http', '--port', '3101'];
73+
const result = parseCliOptions(customArgv);
74+
expect(result.http?.port).toBe(3101);
75+
});
76+
77+
it('trims spaces in list flags', () => {
78+
const argv = ['node', 'cli', '--http', '--allowed-hosts', ' localhost , 127.0.0.1 '];
79+
const result = parseCliOptions(argv);
80+
expect(result.http?.allowedHosts).toEqual(['localhost', '127.0.0.1']);
81+
});
6682
});

‎src/index.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
import { parseCliOptions, type CliOptions, type DefaultOptions } from './options';
2-
import {
3-
getSessionOptions,
4-
setOptions,
5-
runWithSession
6-
} from './options.context';
1+
import { parseCliOptions, type CliOptions, type DefaultOptionsOverrides } from './options';
2+
import { getSessionOptions, setOptions, runWithSession } from './options.context';
73
import {
84
runServer,
95
type ServerInstance,
@@ -24,7 +20,7 @@ import {
2420
* - `'programmatic'`: Functionality is invoked programmatically. Allows process exits.
2521
* - `'test'`: Functionality is being tested. Does NOT allow process exits.
2622
*/
27-
type PfMcpOptions = Partial<DefaultOptions> & {
23+
type PfMcpOptions = DefaultOptionsOverrides & {
2824
mode?: 'cli' | 'programmatic' | 'test';
2925
};
3026

‎src/options.context.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { AsyncLocalStorage } from 'node:async_hooks';
22
import { randomUUID } from 'node:crypto';
3-
import { type Session, type GlobalOptions } from './options';
4-
import { DEFAULT_OPTIONS, LOG_BASENAME, type LoggingSession, type DefaultOptions } from './options.defaults';
3+
import { type AppSession, type GlobalOptions, type DefaultOptionsOverrides } from './options';
4+
import { DEFAULT_OPTIONS, LOG_BASENAME, type LoggingSession } from './options.defaults';
55
import { mergeObjects, freezeObject, isPlainObject } from './server.helpers';
66

77
/**
@@ -10,14 +10,14 @@ import { mergeObjects, freezeObject, isPlainObject } from './server.helpers';
1010
* The `sessionContext` allows sharing a common context without explicitly
1111
* passing it as a parameter.
1212
*/
13-
const sessionContext = new AsyncLocalStorage<Session>();
13+
const sessionContext = new AsyncLocalStorage<AppSession>();
1414

1515
/**
1616
* Initialize and return session data.
1717
*
18-
* @returns {Session} Immutable session with a session ID and channel name.
18+
* @returns {AppSession} Immutable session with a session ID and channel name.
1919
*/
20-
const initializeSession = (): Session => {
20+
const initializeSession = (): AppSession => {
2121
const sessionId = (process.env.NODE_ENV === 'local' && '1234d567-1ce9-123d-1413-a1234e56c789') || randomUUID();
2222
const channelName = `${LOG_BASENAME}:${sessionId}`;
2323

@@ -27,10 +27,10 @@ const initializeSession = (): Session => {
2727
/**
2828
* Set and return the current session options.
2929
*
30-
* @param {Session} [session]
31-
* @returns {Session}
30+
* @param {AppSession} [session]
31+
* @returns {AppSession}
3232
*/
33-
const setSessionOptions = (session: Session = initializeSession()) => {
33+
const setSessionOptions = (session: AppSession = initializeSession()) => {
3434
sessionContext.enterWith(session);
3535

3636
return session;
@@ -39,10 +39,10 @@ const setSessionOptions = (session: Session = initializeSession()) => {
3939
/**
4040
* Get the current session options or set a new session with defaults.
4141
*/
42-
const getSessionOptions = (): Session => sessionContext.getStore() || setSessionOptions();
42+
const getSessionOptions = (): AppSession => sessionContext.getStore() || setSessionOptions();
4343

4444
const runWithSession = async <TReturn>(
45-
session: Session,
45+
session: AppSession,
4646
callback: () => TReturn | Promise<TReturn>
4747
) => {
4848
const frozen = freezeObject(structuredClone(session));
@@ -61,10 +61,10 @@ const optionsContext = new AsyncLocalStorage<GlobalOptions>();
6161
/**
6262
* Set and freeze cloned options in the current async context.
6363
*
64-
* @param {Partial<DefaultOptions>} [options] - Optional options to set in context. Merged with DEFAULT_OPTIONS.
64+
* @param {DefaultOptionsOverrides} [options] - Optional overrides merged with DEFAULT_OPTIONS.
6565
* @returns {GlobalOptions} Cloned frozen default options object with session.
6666
*/
67-
const setOptions = (options?: Partial<DefaultOptions>): GlobalOptions => {
67+
const setOptions = (options?: DefaultOptionsOverrides): GlobalOptions => {
6868
const base = mergeObjects(DEFAULT_OPTIONS, options, { allowNullValues: false, allowUndefinedValues: false });
6969
const baseLogging = isPlainObject(base.logging) ? base.logging : DEFAULT_OPTIONS.logging;
7070
const merged: GlobalOptions = {
@@ -102,7 +102,7 @@ const getOptions = (): GlobalOptions => optionsContext.getStore() || setOptions(
102102
/**
103103
* Get logging options from the current context.
104104
*
105-
* @param {Session} [session] - Session options to use in context.
105+
* @param {AppSession} [session] - Session options to use in context.
106106
* @returns {LoggingSession} Logging options from context.
107107
*/
108108
const getLoggerOptions = (session = getSessionOptions()): LoggingSession => {

‎src/options.defaults.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ interface DefaultOptions<TLogOptions = LoggingOptions> {
3535
contextPath: string;
3636
docsHost: boolean;
3737
docsPath: string;
38-
http: HttpOptions | undefined;
38+
http: HttpOptions;
3939
isHttp: boolean;
4040
llmsFilesPath: string;
4141
logging: TLogOptions;
@@ -57,11 +57,22 @@ interface DefaultOptions<TLogOptions = LoggingOptions> {
5757
version: string;
5858
}
5959

60+
/**
61+
* Overrides for default options.
62+
*/
63+
type DefaultOptionsOverrides = Partial<
64+
Omit<DefaultOptions, 'http' | 'logging'>
65+
> & {
66+
http?: Partial<HttpOptions>;
67+
logging?: Partial<LoggingOptions>;
68+
};
69+
6070
/**
6171
* Logging options.
6272
*
73+
* See `LOGGING_OPTIONS` for defaults.
74+
*
6375
* @interface LoggingOptions
64-
* @default { level: 'debug', logger: packageJson.name, stderr: false, protocol: false, transport: 'stdio' }
6576
*
6677
* @property level Logging level.
6778
* @property logger Logger name. Human-readable/configurable logger name used in MCP protocol messages. Isolated
@@ -82,8 +93,9 @@ interface LoggingOptions {
8293
/**
8394
* HTTP server options.
8495
*
96+
* See `HTTP_OPTIONS` for defaults.
97+
*
8598
* @interface HttpOptions
86-
* @default { port: 8080, host: '127.0.0.1', allowedOrigins: [], allowedHosts: [] }
8799
*
88100
* @property port Port number.
89101
* @property host Host name.
@@ -288,6 +300,7 @@ export {
288300
LOG_BASENAME,
289301
DEFAULT_OPTIONS,
290302
type DefaultOptions,
303+
type DefaultOptionsOverrides,
291304
type HttpOptions,
292305
type LoggingOptions,
293306
type LoggingSession

0 commit comments

Comments
 (0)