Skip to content

Commit ecd2d8a

Browse files
cdcabreracdcabrera
committed
fix: review updates
1 parent 0f44278 commit ecd2d8a

5 files changed

Lines changed: 68 additions & 35 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,14 @@ Unit tests are located in the `__tests__` directory.
127127

128128
E2E tests are located in the root `./tests` directory.
129129

130+
Contributors can run the MCP server in a specialized `test` mode against mock resources.
131+
132+
```bash
133+
npm run test:integration
134+
```
135+
136+
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.
137+
130138
## AI agent
131139

132140
### User Section

‎docs/development.md‎

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,28 @@ Complete guide to using the PatternFly MCP Server for development including CLI
1212

1313
### Available options
1414

15-
| Flag | Description | Default |
16-
|:--------------------------------------|:------------------------------------------------------|:---------------------|
17-
| `--http` | Enable HTTP transport mode | `false` (stdio mode) |
18-
| `--port <num>` | Port for HTTP transport | `8080` |
19-
| `--host <string>` | Host to bind to | `127.0.0.1` |
20-
| `--allowed-origins <origins>` | Comma-separated list of allowed CORS origins | `none` |
21-
| `--allowed-hosts <hosts>` | Comma-separated list of allowed host headers | `none` |
22-
| `--tool <path>` | Path to external Tool Plugin (repeatable) | `none` |
23-
| `--plugin-isolation <none \| strict>` | Isolation preset for external tools-as-plugins | `strict` |
24-
| `--log-stderr` | Enable terminal logging | `false` |
25-
| `--log-protocol` | Forward logs to MCP clients | `false` |
26-
| `--log-level <level>` | Set log level (`debug`, `info`, `warn`, `error`) | `info` |
27-
| `--verbose` | Shortcut for `--log-level debug` | `false` |
15+
| Flag | Description | Default |
16+
|:--------------------------------------|:-------------------------------------------------|:---------------------|
17+
| `--http` | Enable HTTP transport mode | `false` (stdio mode) |
18+
| `--port <num>` | Port for HTTP transport | `8080` |
19+
| `--host <string>` | Host to bind to | `127.0.0.1` |
20+
| `--allowed-origins <origins>` | Comma-separated list of allowed CORS origins | `none` |
21+
| `--allowed-hosts <hosts>` | Comma-separated list of allowed host headers | `none` |
22+
| `--tool <path>` | Path to external Tool Plugin (repeatable) | `none` |
23+
| `--plugin-isolation <none \| strict>` | Isolation preset for external tools-as-plugins | `strict` |
24+
| `--log-stderr` | Enable terminal logging | `false` |
25+
| `--log-protocol` | Forward logs to MCP clients | `false` |
26+
| `--log-level <level>` | Set log level (`debug`, `info`, `warn`, `error`) | `info` |
27+
| `--mode <mode>` | Operational mode (`cli`, `programmatic`, `test`) | `cli` |
28+
| `--mode-test-url <url>` | Base URL for fixture/mock servers in `test` mode | `none` |
29+
| `--verbose` | Shortcut for `--log-level debug` | `false` |
2830

2931
#### Notes
3032
- **HTTP transport mode** - By default, the server uses `stdio`. Use the `--http` flag to enable HTTP transport.
3133
- **Logging** - The server uses a `diagnostics_channel`-based logger that keeps STDIO stdout pure by default.
3234
- **Programmatic API** - The server can also be used programmatically with options. See [Programmatic Usage](#programmatic-usage) for more details.
33-
- **Tool Plugins** - The server can load external tool plugins at startup. See [Tool Plugins](#tool-plugins) for more details.
35+
- **Tool Plugins** - The server can load external tool plugins at startup. See [Tool Plugins](#tool-plugins) for more details.
36+
- **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.
3437

3538
### Basic use scenarios
3639

@@ -53,6 +56,10 @@ npx @patternfly/patternfly-mcp --http --port 3000 --allowed-origins "https://app
5356
```bash
5457
npx @patternfly/patternfly-mcp --tool ./first-tool.js --tool ./second-tool.ts
5558
```
59+
**Testing with a fixture server**:
60+
```bash
61+
npx @patternfly/patternfly-mcp --mode test --mode-test-url "http://localhost:3000"
62+
```
5663

5764
### Testing with MCP Inspector
5865

@@ -81,20 +88,21 @@ npx @modelcontextprotocol/inspector-cli \
8188

8289
The `start()` function accepts an optional `PfMcpOptions` object for programmatic configuration. Use these options to customize behavior, transport, and logging for embedded instances.
8390

84-
| Option | Type | Description | Default |
85-
|:----------------------|:-----------------------------------------|:----------------------------------------------------------------------|:-------------------|
86-
| `toolModules` | `ToolModule \| ToolModule[]` | Array of tool modules or paths to external tool plugins to be loaded. | `[]` |
87-
| `isHttp` | `boolean` | Enable HTTP transport mode. | `false` |
88-
| `http.port` | `number` | Port for HTTP transport. | `8080` |
89-
| `http.host` | `string` | Host to bind to. | `127.0.0.1` |
90-
| `http.allowedOrigins` | `string[]` | List of allowed CORS origins. | `[]` |
91-
| `http.allowedHosts` | `string[]` | List of allowed host headers. | `[]` |
92-
| `pluginIsolation` | `'none' \| 'strict'` | Isolation preset for external tools-as-plugins. | `'strict'` |
93-
| `logging.level` | `'debug' \| 'info' \| 'warn' \| 'error'` | Set the logging level. | `'info'` |
94-
| `logging.stderr` | `boolean` | Enable terminal logging to stderr. | `false` |
95-
| `logging.protocol` | `boolean` | Forward logs to MCP clients. | `false` |
96-
| `mode` | `'cli' \| 'programmatic' \| 'test'` | Specifies the operation mode. | `'programmatic'` |
97-
| `docsPath` | `string` | Path to the documentation directory. | (Internal default) |
91+
| Option | Type | Description | Default |
92+
|:---------------------------|:-----------------------------------------|:----------------------------------------------------------------------|:-------------------|
93+
| `toolModules` | `ToolModule \| ToolModule[]` | Array of tool modules or paths to external tool plugins to be loaded. | `[]` |
94+
| `isHttp` | `boolean` | Enable HTTP transport mode. | `false` |
95+
| `http.port` | `number` | Port for HTTP transport. | `8080` |
96+
| `http.host` | `string` | Host to bind to. | `127.0.0.1` |
97+
| `http.allowedOrigins` | `string[]` | List of allowed CORS origins. | `[]` |
98+
| `http.allowedHosts` | `string[]` | List of allowed host headers. | `[]` |
99+
| `pluginIsolation` | `'none' \| 'strict'` | Isolation preset for external tools-as-plugins. | `'strict'` |
100+
| `logging.level` | `'debug' \| 'info' \| 'warn' \| 'error'` | Set the logging level. | `'info'` |
101+
| `logging.stderr` | `boolean` | Enable terminal logging to stderr. | `false` |
102+
| `logging.protocol` | `boolean` | Forward logs to MCP clients. | `false` |
103+
| `mode` | `'cli' \| 'programmatic' \| 'test'` | Specifies the operation mode. | `'programmatic'` |
104+
| `modeOptions.test.baseUrl` | `string` | Base URL for fixture/mock servers in `test` mode. | `undefined` |
105+
| `docsPath` | `string` | Path to the documentation directory. | (Internal default) |
98106

99107
#### Example usage
100108

@@ -116,6 +124,20 @@ const options: PfMcpOptions = {
116124
const server: PfMcpInstance = await start(options);
117125
```
118126

127+
**Example: Programmatic test mode**
128+
```typescript
129+
import { start, type PfMcpInstance } from '@patternfly/patternfly-mcp';
130+
131+
const server: PfMcpInstance = await start({
132+
mode: 'test',
133+
modeOptions: {
134+
test: {
135+
baseUrl: 'http://my-fixture-server:3000'
136+
}
137+
}
138+
});
139+
```
140+
119141
### Server instance
120142

121143
The server instance exposes the following methods:

‎src/index.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,15 @@ const main = async (
156156
let updatedAllowProcessExit = allowProcessExit ?? programmaticMode !== 'test';
157157
let mergedOptions: ServerOptions;
158158

159-
// If allowed, exit the process on error
159+
// If allowed, exit the process on error otherwise log then throw the error.
160160
const processExit = (message: string, error: unknown) => {
161161
console.error(message, error);
162162

163163
if (updatedAllowProcessExit) {
164164
process.exit(1);
165165
}
166+
167+
throw error;
166168
};
167169

168170
try {
@@ -176,7 +178,6 @@ const main = async (
176178
updatedAllowProcessExit = allowProcessExit ?? mergedOptions.mode !== 'test';
177179
} catch (error) {
178180
processExit('Set options error, failed to start server:', error);
179-
throw error;
180181
}
181182

182183
try {
@@ -188,8 +189,9 @@ const main = async (
188189
await runServer.memo(mergedOptions, { allowProcessExit: updatedAllowProcessExit }));
189190
} catch (error) {
190191
processExit('Failed to start server:', error);
191-
throw error;
192192
}
193+
194+
return undefined as never;
193195
};
194196

195197
export {

‎src/options.defaults.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,6 @@ const LOG_BASENAME = 'pf-mcp:log';
348348
* Default PatternFly-specific options.
349349
*/
350350
const PATTERNFLY_OPTIONS: PatternFlyOptions = {
351-
// availableVersions: ['v3', 'v4', 'v5', 'v6'],
352351
availableResourceVersions: ['v6'],
353352
default: {
354353
defaultVersion: 'v6',

‎src/server.getResources.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ const resolveLocalPathFunction = (path: string, options = getOptions()) => {
7878
const documentationPrefix = options.docsPathSlug;
7979

8080
// Safety check: Ensure the path is within the allowed directory
81-
const confirmThenReturnResolvedBase = (base: string, resolved: string) => {
81+
const assertPathWithinBaseAndReturn = (base: string, resolved: string) => {
8282
const normalizedBase = normalize(base);
8383
const refinedBase = normalizedBase.endsWith(sep) ? normalizedBase : `${normalizedBase}${sep}`;
8484

@@ -89,21 +89,23 @@ const resolveLocalPathFunction = (path: string, options = getOptions()) => {
8989
return resolved;
9090
};
9191

92+
// Paths starting with the documentation prefix are resolved relative to the documentation path
9293
if (path.startsWith(documentationPrefix)) {
9394
const base = options.docsPath;
9495
const resolved = resolve(base, path.slice(documentationPrefix.length));
9596

96-
return confirmThenReturnResolvedBase(base, resolved);
97+
return assertPathWithinBaseAndReturn(base, resolved);
9798
}
9899

100+
// URLs are returned as-is
99101
if (isUrl(path)) {
100102
return path;
101103
}
102104

103105
const base = options.contextPath;
104106
const resolved = isAbsolute(path) ? normalize(path) : resolve(base, path);
105107

106-
return confirmThenReturnResolvedBase(base, resolved);
108+
return assertPathWithinBaseAndReturn(base, resolved);
107109
};
108110

109111
/**

0 commit comments

Comments
 (0)