From 57e2f52fd5f0d293a6caaba3a3b4cf375d570988 Mon Sep 17 00:00:00 2001 From: modesty Date: Wed, 7 Jan 2026 14:31:56 -0800 Subject: [PATCH 1/2] feat: add mcpErrors with protocol compliant error code, also add more tool input validation --- src/server/fluentMCPServer.ts | 98 +++++++++++++---------- src/tools/cliCommandTools.ts | 22 ++--- src/tools/commands/baseCommand.ts | 78 +++++++++++++++++- src/tools/commands/sessionAwareCommand.ts | 29 +++---- src/utils/mcpErrors.ts | 95 ++++++++++++++++++++++ src/utils/rootContext.ts | 28 +++++++ test/tools/sessionAwareCommand.test.ts | 21 +++-- 7 files changed, 289 insertions(+), 82 deletions(-) create mode 100644 src/utils/mcpErrors.ts diff --git a/src/server/fluentMCPServer.ts b/src/server/fluentMCPServer.ts index 683e01d..0a5272d 100644 --- a/src/server/fluentMCPServer.ts +++ b/src/server/fluentMCPServer.ts @@ -23,6 +23,7 @@ import { PromptManager } from '../prompts/promptManager.js'; import { autoValidateAuthIfConfigured } from './fluentInstanceAuth.js'; import { SamplingManager } from '../utils/samplingManager.js'; import { AuthNotificationHandler } from './authNotificationHandler.js'; +import { McpResourceNotFoundError, McpUnknownToolError, McpInternalError } from '../utils/mcpErrors.js'; /** Delay before fallback initialization if client doesn't send notifications */ const INITIALIZATION_DELAY_MS = 1000; @@ -108,12 +109,12 @@ export class FluentMcpServer { return `✅ Output:\n${result.output}`; } else { let errorOutput = `❌ Error:\n${result.error || 'Unknown error'}\n(exit code: ${result.exitCode})\n${result.output}`; - + // Append AI error analysis if available if (result.errorAnalysis) { errorOutput += '\n' + this.samplingManager.formatAnalysis(result.errorAnalysis); } - + return errorOutput; } } @@ -136,7 +137,7 @@ export class FluentMcpServer { // Only initialize if roots haven't been set up yet if (this.roots.length === 0) { logger.info('No roots received from client after delay, using fallback initialization...'); - + // Try to request from client first, with short timeout try { await Promise.race([ @@ -152,7 +153,7 @@ export class FluentMcpServer { await this.addRoot(`file://${projectRoot}`, 'Project Root (Fallback)'); } } - + // Trigger auth validation if not already triggered if (!this.autoAuthTriggered) { this.autoAuthTriggered = true; @@ -187,7 +188,7 @@ export class FluentMcpServer { } logger.info('Requesting roots from client via roots/list...'); - + try { // Create a schema for the response using the Zod library // This is needed because the request method requires a result schema @@ -208,15 +209,15 @@ export class FluentMcpServer { ) as RootsResponse; const roots = response.roots; - + if (Array.isArray(roots) && roots.length > 0) { logger.info('Received roots from client', { rootCount: roots.length }); - + // Update roots with client-provided roots await this.updateRoots(roots); } else { logger.warn('Client responded to roots/list but provided no roots'); - + // Fall back to project root if no valid roots received if (this.roots.length === 0) { const projectRoot = getProjectRootPath(); @@ -225,10 +226,10 @@ export class FluentMcpServer { } } } catch (error) { - logger.error('Error requesting roots from client', + logger.error('Error requesting roots from client', error instanceof Error ? error : new Error(String(error)) ); - + // Fall back to project root if request fails if (this.roots.length === 0) { const projectRoot = getProjectRootPath(); @@ -244,15 +245,15 @@ export class FluentMcpServer { private setupHandlers(): void { const server = this.mcpServer?.server; if (!server) return; - + // Set up the tools/list handler server.setRequestHandler(ListToolsRequestSchema, async () => { const tools = this.toolsManager.getMCPTools(); - + // Start a delayed initialization process to ensure roots and auth are set up // even if the client doesn't send proper notifications this.scheduleDelayedInitialization(); - + return { tools }; }); @@ -266,31 +267,46 @@ export class FluentMcpServer { return { resources: [] }; } }); - + // Set up the resources/read handler // The SDK's registerResource() doesn't automatically set up the read handler // We need to explicitly handle resource read requests server.setRequestHandler(ReadResourceRequestSchema, async (request) => { const { uri } = request.params; - + try { logger.debug('Reading resource', { uri }); - + // Call ResourceManager to handle the read request const result = await this.resourceManager.readResource(uri); + + // Check if resource was not found (result has no contents) + if (!result.contents || result.contents.length === 0) { + throw new McpResourceNotFoundError(uri); + } + return result; } catch (error) { + // Re-throw MCP errors as-is for proper error codes + if (error instanceof McpResourceNotFoundError) { + logger.warn('Resource not found', { uri }); + throw error; + } + + // Wrap other errors as internal errors logger.error('Error reading resource', error instanceof Error ? error : new Error(String(error)), { uri } ); - throw error; + throw new McpInternalError( + `Failed to read resource: ${error instanceof Error ? error.message : String(error)}` + ); } }); - + // Set up prompts handlers this.promptManager.setupHandlers(); - + // Set up roots/list handler server.setRequestHandler(ListRootsRequestSchema, async () => { logger.debug('Received roots/list request, returning current roots'); @@ -298,7 +314,7 @@ export class FluentMcpServer { roots: this.roots, }; }); - + // Set up handler for notifications/initialized server.setNotificationHandler(InitializedNotificationSchema, async () => { logger.info('Received notifications/initialized notification from client'); @@ -336,28 +352,28 @@ export class FluentMcpServer { } } }); - + // Set up handler for roots/list_changed notification server.setNotificationHandler(RootsListChangedNotificationSchema, async () => { logger.info('Received notifications/roots/list_changed notification from client'); - + // When a root list change notification is received, request the updated roots list try { await this.requestRootsFromClient(); } catch (error) { - logger.error('Failed to request updated roots after notification', + logger.error('Failed to request updated roots after notification', error instanceof Error ? error : new Error(String(error)) ); } }); - + // Execute tool calls handler server.setRequestHandler(CallToolRequestSchema, async (request) => { const { name, arguments: args } = request.params; const command = this.toolsManager.getCommand(name); if (!command) { - throw new Error(`Unknown command: ${name}`); + throw new McpUnknownToolError(name); } try { @@ -366,10 +382,10 @@ export class FluentMcpServer { // If command failed and error analysis is enabled, analyze the error if (!result.success && this.config.sampling.enableErrorAnalysis && result.error) { const errorMessage = result.error.message; - + if (this.samplingManager.shouldAnalyzeError(errorMessage, this.config.sampling.minErrorLength)) { logger.info('Triggering error analysis for failed command', { command: name }); - + try { const analysis = await this.samplingManager.analyzeError({ command: name, @@ -377,7 +393,7 @@ export class FluentMcpServer { errorOutput: errorMessage, exitCode: result.exitCode, }); - + if (analysis) { result.errorAnalysis = analysis; } @@ -428,17 +444,17 @@ export class FluentMcpServer { } return true; }); - + // Check if roots have changed const hasChanged = this.roots.length !== validatedRoots.length || - this.roots.some((root, index) => - root.uri !== validatedRoots[index]?.uri || + this.roots.some((root, index) => + root.uri !== validatedRoots[index]?.uri || root.name !== validatedRoots[index]?.name ); - + if (hasChanged) { this.roots = [...validatedRoots]; - + // Only update tools manager with the roots if the server is running // or if the status is INITIALIZING (for tests) // This prevents unnecessary updates during initialization @@ -459,7 +475,7 @@ export class FluentMcpServer { } } } - + /** * Add a new root to the list of roots * @param uri The URI of the root @@ -471,10 +487,10 @@ export class FluentMcpServer { logger.warn('Attempted to add root with empty URI, ignoring'); return; } - + // Check if root already exists const existingIndex = this.roots.findIndex(root => root.uri === uri); - + if (existingIndex >= 0) { // Update existing root if name has changed if (this.roots[existingIndex].name !== name) { @@ -491,19 +507,19 @@ export class FluentMcpServer { await this.updateRoots([...this.roots, { uri, name }]); } } - + /** * Remove a root from the list of roots * @param uri The URI of the root to remove */ async removeRoot(uri: string): Promise { const updatedRoots = this.roots.filter(root => root.uri !== uri); - + if (updatedRoots.length !== this.roots.length) { await this.updateRoots(updatedRoots); } } - + /** * Get the current list of roots * @returns The list of roots @@ -511,7 +527,7 @@ export class FluentMcpServer { getRoots(): { uri: string; name?: string }[] { return [...this.roots]; } - + /** * Start the MCP server */ @@ -552,7 +568,7 @@ export class FluentMcpServer { // This ensures that client notifications will be sent correctly this.status = ServerStatus.RUNNING; loggingManager.logServerStarted(); - + // The root list will be requested when the client sends the notifications/initialized notification // This ensures proper timing according to the MCP protocol } catch (error) { diff --git a/src/tools/cliCommandTools.ts b/src/tools/cliCommandTools.ts index 2149c12..0eccf13 100644 --- a/src/tools/cliCommandTools.ts +++ b/src/tools/cliCommandTools.ts @@ -9,7 +9,7 @@ import { ProcessResult, ProcessRunner, } from '../utils/types.js'; -import { getPrimaryRootPath as getRootContextPrimaryRootPath, getPrimaryRootPathFrom as getPrimaryRootPathFromArray } from '../utils/rootContext.js'; +import { resolveWorkingDirectory } from '../utils/rootContext.js'; import { SdkInfoCommand, InitCommand, @@ -233,9 +233,8 @@ export class CLIExecutor extends BaseCommandProcessor { try { let cwd = customWorkingDir; if (!cwd && useMcpCwd) { - // Prefer instance roots when provided, fallback to RootContext - const resolved = getPrimaryRootPathFromArray(this.roots); - cwd = resolved || getRootContextPrimaryRootPath(); + // Use canonical working directory resolution + cwd = resolveWorkingDirectory(this.roots); } // Sanity check on working directory - warn if it's the system root @@ -307,9 +306,10 @@ export class CLICmdWriter extends BaseCommandProcessor { try { let cwd = customWorkingDir; if (!cwd && useMcpCwd) { - cwd = getPrimaryRootPathFromArray(this.roots) || getRootContextPrimaryRootPath(); + // Use canonical working directory resolution + cwd = resolveWorkingDirectory(this.roots); } - + // Sanity check on working directory - warn if it's the system root if (cwd === '/' || cwd === '\\') { throw new Error('ERROR: Command should never use system root (/) as working directory'); @@ -318,11 +318,11 @@ export class CLICmdWriter extends BaseCommandProcessor { // Format the command string const argsText = args.join(' '); const commandText = `${command} ${argsText}`; - + // Add working directory context if available const cwdInfo = cwd ? `(in directory: ${cwd})` : ''; const fullCommandText = cwdInfo ? `${commandText} ${cwdInfo}` : commandText; - + logger.info(`Generated command text: ${fullCommandText}`); return CommandResultFactory.success(fullCommandText); @@ -352,7 +352,7 @@ export class CommandRegistry { // Convert to MCP Tool format toMCPTools(): Tool[] { - return this.getAllCommands().map((command) => { + return this.getAllCommands().map((command) => { const tool: Tool = { name: command.name, description: command.description, @@ -375,7 +375,7 @@ export class CommandRegistry { .map((arg) => arg.name), } }; - + // Add annotations if they exist if (command.annotations) { // MCP SDK expects annotations to be a direct object with properties @@ -387,7 +387,7 @@ export class CommandRegistry { openWorldHint: command.annotations.openWorldHint }; } - + return tool; }); } diff --git a/src/tools/commands/baseCommand.ts b/src/tools/commands/baseCommand.ts index f128c99..aa64a03 100644 --- a/src/tools/commands/baseCommand.ts +++ b/src/tools/commands/baseCommand.ts @@ -1,4 +1,10 @@ -import { CLICommand, CommandArgument, CommandProcessor, CommandResult } from '../../utils/types'; +import { CLICommand, CommandArgument, CommandProcessor, CommandResult } from '../../utils/types.js'; + +/** + * Dangerous shell characters that could enable command injection + * These patterns are rejected in string arguments to prevent security vulnerabilities + */ +const DANGEROUS_SHELL_PATTERN = /[;&|`$(){}[\]<>\\]/; /** * Base abstract class for all CLI commands @@ -9,7 +15,7 @@ export abstract class BaseCLICommand implements CLICommand { abstract description: string; abstract arguments: CommandArgument[]; - constructor(protected commandProcessor: CommandProcessor) {} + constructor(protected commandProcessor: CommandProcessor) { } abstract execute(args: Record): Promise; @@ -21,12 +27,78 @@ export abstract class BaseCLICommand implements CLICommand { return this.commandProcessor; } - // Template method for common validation + /** + * Sanitize a string argument to prevent command injection + * @param value The value to sanitize + * @param argName The argument name (for error messages) + * @returns The sanitized string + * @throws Error if the value contains dangerous characters + */ + protected sanitizeStringArg(value: unknown, argName: string): string { + const str = String(value); + if (DANGEROUS_SHELL_PATTERN.test(str)) { + const truncated = str.length > 50 ? `${str.substring(0, 50)}...` : str; + throw new Error( + `Invalid characters in argument '${argName}': "${truncated}". ` + + 'Shell metacharacters are not allowed for security reasons.' + ); + } + return str; + } + + /** + * Validate and sanitize command arguments + * - Checks required arguments are present + * - Validates argument types match expected types + * - Sanitizes string arguments to prevent command injection + * @param args The arguments to validate + * @throws Error if validation fails + */ protected validateArgs(args: Record): void { for (const arg of this.arguments) { + const value = args[arg.name]; + + // Check required arguments are present if (arg.required && !(arg.name in args)) { throw new Error(`Required argument '${arg.name}' is missing`); } + + // Skip validation for undefined optional arguments + if (value === undefined || value === null) { + continue; + } + + // Validate type and sanitize + switch (arg.type) { + case 'string': + if (typeof value !== 'string') { + throw new Error(`Argument '${arg.name}' must be a string, got ${typeof value}`); + } + // Sanitize string arguments to prevent command injection + this.sanitizeStringArg(value, arg.name); + break; + case 'number': + if (typeof value !== 'number' || isNaN(value)) { + throw new Error(`Argument '${arg.name}' must be a valid number`); + } + break; + case 'boolean': + if (typeof value !== 'boolean') { + throw new Error(`Argument '${arg.name}' must be a boolean`); + } + break; + case 'array': + if (!Array.isArray(value)) { + throw new Error(`Argument '${arg.name}' must be an array`); + } + // Sanitize array elements if they are strings + for (const item of value) { + if (typeof item === 'string') { + this.sanitizeStringArg(item, `${arg.name}[]`); + } + } + break; + } } } diff --git a/src/tools/commands/sessionAwareCommand.ts b/src/tools/commands/sessionAwareCommand.ts index 4593faa..64fcee8 100644 --- a/src/tools/commands/sessionAwareCommand.ts +++ b/src/tools/commands/sessionAwareCommand.ts @@ -1,7 +1,7 @@ import { CommandResult, CommandResultFactory } from '../../utils/types.js'; import { BaseCLICommand } from './baseCommand.js'; import { SessionManager } from '../../utils/sessionManager.js'; -import { getPrimaryRootPath } from '../../utils/rootContext.js'; +import { resolveWorkingDirectory } from '../../utils/rootContext.js'; import logger from '../../utils/logger.js'; /** @@ -10,26 +10,23 @@ import logger from '../../utils/logger.js'; */ export abstract class SessionAwareCLICommand extends BaseCLICommand { /** - * Get the working directory from the session, or use the root context as fallback + * Get the working directory from the session, or use the root context as fallback. + * Priority: session working directory > root context > project root * @returns The working directory to use for the command */ protected getWorkingDirectory(): string | undefined { const sessionManager = SessionManager.getInstance(); - let workingDirectory = sessionManager.getWorkingDirectory(); - - if (workingDirectory) { - logger.debug(`Using session working directory: ${workingDirectory}`); - } else { - // Fallback to root context when no session working directory is set - try { - workingDirectory = getPrimaryRootPath(); - logger.debug(`No session working directory found, using root context: ${workingDirectory}`); - } catch (error) { - logger.debug(`No working directory found in session or root context: ${error}`); - } + const sessionDir = sessionManager.getWorkingDirectory(); + + if (sessionDir) { + logger.debug(`Using session working directory: ${sessionDir}`); + return sessionDir; } - - return workingDirectory; + + // Fallback to root context using canonical resolution + const fallbackDir = resolveWorkingDirectory(); + logger.debug(`No session working directory, using root context: ${fallbackDir}`); + return fallbackDir; } /** diff --git a/src/utils/mcpErrors.ts b/src/utils/mcpErrors.ts new file mode 100644 index 0000000..e921878 --- /dev/null +++ b/src/utils/mcpErrors.ts @@ -0,0 +1,95 @@ +/** + * MCP-compliant error classes with proper JSON-RPC error codes + * + * Standard JSON-RPC error codes used by MCP: + * - -32602: Invalid params + * - -32002: Resource not found + * - -32603: Internal error + * + * @see https://modelcontextprotocol.io/docs/concepts/resources#error-handling + * @see https://modelcontextprotocol.io/docs/concepts/tools#error-handling + */ + +/** + * Base class for MCP protocol errors + */ +export abstract class McpError extends Error { + abstract readonly code: number; + + constructor(message: string) { + super(message); + this.name = this.constructor.name; + // Maintains proper stack trace for where error was thrown (V8 only) + if (Error.captureStackTrace) { + Error.captureStackTrace(this, this.constructor); + } + } + + /** + * Convert to JSON-RPC error format + */ + toJsonRpcError(): { code: number; message: string; data?: unknown } { + return { + code: this.code, + message: this.message, + }; + } +} + +/** + * Error thrown when a requested resource is not found + * JSON-RPC error code: -32002 + */ +export class McpResourceNotFoundError extends McpError { + readonly code = -32002; + readonly uri: string; + + constructor(uri: string, details?: string) { + super(details ? `Resource not found: ${uri} - ${details}` : `Resource not found: ${uri}`); + this.uri = uri; + } + + toJsonRpcError(): { code: number; message: string; data?: { uri: string } } { + return { + code: this.code, + message: this.message, + data: { uri: this.uri }, + }; + } +} + +/** + * Error thrown when tool/method parameters are invalid + * JSON-RPC error code: -32602 + */ +export class McpInvalidParamsError extends McpError { + readonly code = -32602; + + constructor(message: string) { + super(`Invalid params: ${message}`); + } +} + +/** + * Error thrown for internal server errors + * JSON-RPC error code: -32603 + */ +export class McpInternalError extends McpError { + readonly code = -32603; + + constructor(message: string, public readonly cause?: Error) { + super(`Internal error: ${message}`); + } +} + +/** + * Error thrown when a tool is not found + * JSON-RPC error code: -32602 (per MCP spec, unknown tools use invalid params code) + */ +export class McpUnknownToolError extends McpError { + readonly code = -32602; + + constructor(toolName: string) { + super(`Unknown tool: ${toolName}`); + } +} diff --git a/src/utils/rootContext.ts b/src/utils/rootContext.ts index d12684f..42ddd35 100644 --- a/src/utils/rootContext.ts +++ b/src/utils/rootContext.ts @@ -40,3 +40,31 @@ export function getPrimaryRootPathFrom( ): string { return resolveUriToPath(providedRoots?.[0]?.uri) ?? getProjectRootPath(); } + +/** + * Resolve the working directory from instance roots, falling back to global roots and project root. + * This is the canonical way to determine the working directory for CLI commands. + * + * Resolution order: + * 1. Instance-specific roots (if provided and non-empty) + * 2. Global roots from RootContext + * 3. Project root as final fallback + * + * @param instanceRoots Optional instance-specific roots to prefer over global roots + * @returns The resolved working directory path + */ +export function resolveWorkingDirectory( + instanceRoots?: { uri: string; name?: string }[] +): string { + // Try instance roots first + if (instanceRoots && instanceRoots.length > 0) { + const resolved = resolveUriToPath(instanceRoots[0]?.uri); + if (resolved) { + return resolved; + } + } + + // Fall back to global roots, then project root + return getPrimaryRootPath(); +} + diff --git a/test/tools/sessionAwareCommand.test.ts b/test/tools/sessionAwareCommand.test.ts index 486e9db..f53c6ae 100644 --- a/test/tools/sessionAwareCommand.test.ts +++ b/test/tools/sessionAwareCommand.test.ts @@ -20,6 +20,7 @@ jest.mock("../../src/utils/sessionManager.js", () => { jest.mock("../../src/utils/rootContext.js", () => { return { getPrimaryRootPath: jest.fn().mockReturnValue("/test-root-context"), + resolveWorkingDirectory: jest.fn().mockReturnValue("/test-root-context"), }; }); @@ -41,7 +42,7 @@ describe("SessionAwareCommand", () => { beforeEach(() => { jest.clearAllMocks(); - + // Create a mock processor mockExecutor = { process: jest.fn().mockImplementation(() => { @@ -65,7 +66,7 @@ describe("SessionAwareCommand", () => { test("should execute command with working directory from session", async () => { const result = await sessionAwareCommand.execute({}); - + expect(SessionManager.getInstance().getWorkingDirectory).toHaveBeenCalled(); expect(mockExecutor.process).toHaveBeenCalledWith( "test-command", @@ -79,14 +80,12 @@ describe("SessionAwareCommand", () => { test("should return error when no working directory is available", async () => { // Override the mock to return undefined working directory (SessionManager.getInstance().getWorkingDirectory as jest.Mock).mockReturnValueOnce(undefined); - // Mock rootContext to throw error - const { getPrimaryRootPath } = require("../../src/utils/rootContext.js"); - (getPrimaryRootPath as jest.Mock).mockImplementationOnce(() => { - throw new Error("No root context available"); - }); - + // Mock resolveWorkingDirectory to return undefined (simulating no fallback available) + const { resolveWorkingDirectory } = require("../../src/utils/rootContext.js"); + (resolveWorkingDirectory as jest.Mock).mockReturnValueOnce(undefined); + const result = await sessionAwareCommand.execute({}); - + expect(SessionManager.getInstance().getWorkingDirectory).toHaveBeenCalled(); expect(mockExecutor.process).not.toHaveBeenCalled(); expect(result.success).toBe(false); @@ -98,9 +97,9 @@ describe("SessionAwareCommand", () => { (mockExecutor.process as jest.Mock).mockImplementationOnce(() => { throw new Error("Test execution error"); }); - + const result = await sessionAwareCommand.execute({}); - + expect(result.success).toBe(false); expect(result.error?.message).toBe("Test execution error"); }); From f807040c485c014b90deb1fdaa3529242863213d Mon Sep 17 00:00:00 2001 From: modesty Date: Wed, 7 Jan 2026 16:11:57 -0800 Subject: [PATCH 2/2] fix: resolve tool execution timeouts and enforce MCP protocol compliance --- README.md | 4 +- package.json | 2 +- src/server/fluentMCPServer.ts | 68 +-------------- src/tools/commands/sdkInfoCommand.ts | 46 +++++++--- src/tools/resourceTools.ts | 88 ++++++++++--------- src/tools/toolsManager.ts | 56 +++++++----- test/tools/newCommands.test.ts | 61 +++++++------ test/tools/resourceTools.test.ts | 123 +++++++++------------------ test/tools/sdkCommands.test.ts | 26 ++---- 9 files changed, 200 insertions(+), 274 deletions(-) diff --git a/README.md b/README.md index baf801e..a4c8cba 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,8 @@ Create a new Fluent app in ~/projects/time-off-tracker to manage employee PTO re | Tool | Description | Key Parameters | |------|-------------|----------------| -| `sdk_info` | Get SDK version, help, or debug info | `flag` (-v/-h/-d), `command` (optional) | +| `sdk_info` | Get SDK version or help | `flag` (-v/-h), `command` (optional for -h) | +| `get-api-spec` | Get API spec or list all metadata types | `metadataType` (optional, omit to list all) | | `init_fluent_app` | Initialize or convert ServiceNow app | `workingDirectory` (required), `template`, `from` (optional) | | `build_fluent_app` | Build the application | `debug` (optional) | | `deploy_fluent_app` | Deploy to ServiceNow instance | `auth` (auto-injected), `debug` (optional) | @@ -224,6 +225,7 @@ npm run build && npm run inspect - Version command returns SDK version string - Help command returns detailed command documentation +- List metadata (`-lm`) returns available Fluent metadata types - No errors in notifications pane - Commands execute within 2-3 seconds diff --git a/package.json b/package.json index bea0052..1dfd61b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@modesty/fluent-mcp", - "version": "0.0.19", + "version": "0.1.0", "description": "MCP server for Fluent (ServiceNow SDK)", "keywords": [ "Servicenow SDK", diff --git a/src/server/fluentMCPServer.ts b/src/server/fluentMCPServer.ts index 0a5272d..6617802 100644 --- a/src/server/fluentMCPServer.ts +++ b/src/server/fluentMCPServer.ts @@ -2,9 +2,7 @@ import { z } from 'zod'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { - CallToolRequestSchema, ListToolsRequestSchema, - CallToolResult, ListResourcesRequestSchema, ReadResourceRequestSchema, ListRootsRequestSchema, @@ -23,7 +21,7 @@ import { PromptManager } from '../prompts/promptManager.js'; import { autoValidateAuthIfConfigured } from './fluentInstanceAuth.js'; import { SamplingManager } from '../utils/samplingManager.js'; import { AuthNotificationHandler } from './authNotificationHandler.js'; -import { McpResourceNotFoundError, McpUnknownToolError, McpInternalError } from '../utils/mcpErrors.js'; +import { McpResourceNotFoundError, McpInternalError } from '../utils/mcpErrors.js'; /** Delay before fallback initialization if client doesn't send notifications */ const INITIALIZATION_DELAY_MS = 1000; @@ -367,68 +365,8 @@ export class FluentMcpServer { } }); - // Execute tool calls handler - server.setRequestHandler(CallToolRequestSchema, async (request) => { - const { name, arguments: args } = request.params; - - const command = this.toolsManager.getCommand(name); - if (!command) { - throw new McpUnknownToolError(name); - } - - try { - const result = await command.execute(args || {}); - - // If command failed and error analysis is enabled, analyze the error - if (!result.success && this.config.sampling.enableErrorAnalysis && result.error) { - const errorMessage = result.error.message; - - if (this.samplingManager.shouldAnalyzeError(errorMessage, this.config.sampling.minErrorLength)) { - logger.info('Triggering error analysis for failed command', { command: name }); - - try { - const analysis = await this.samplingManager.analyzeError({ - command: name, - args: Object.entries(args || {}).map(([key, value]) => `${key}=${value}`), - errorOutput: errorMessage, - exitCode: result.exitCode, - }); - - if (analysis) { - result.errorAnalysis = analysis; - } - } catch (analysisError) { - // Log but don't fail the tool call if analysis fails - logger.warn('Error analysis failed', { - error: analysisError instanceof Error ? analysisError.message : String(analysisError), - }); - } - } - } - - return { - content: [ - { - type: 'text', - text: this.formatResult(result), - }, - ], - } as CallToolResult; - } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); - - return { - content: [ - { - type: 'text', - text: `Error: ${errorMessage}`, - }, - ], - isError: true, - } as CallToolResult; - } - }); + // Note: Tool calls are handled by the callbacks registered via mcpServer.registerTool() in ToolsManager. + // We don't need a separate setRequestHandler for CallToolRequestSchema as that would conflict. } /** diff --git a/src/tools/commands/sdkInfoCommand.ts b/src/tools/commands/sdkInfoCommand.ts index 2cdc265..8fb12d7 100644 --- a/src/tools/commands/sdkInfoCommand.ts +++ b/src/tools/commands/sdkInfoCommand.ts @@ -1,20 +1,22 @@ -import { CommandArgument, CommandResult, CommandResultFactory } from '../../utils/types'; +import { CommandArgument, CommandResult, CommandResultFactory } from '../../utils/types.js'; import { BaseCLICommand } from './baseCommand.js'; import { getProjectRootPath } from '../../config.js'; /** * ServiceNow SDK information command that accepts SDK flags - * Handles -v/--version, -h/--help, -d/--debug flags as per SDK specification + * Handles: + * - -v/--version: Show SDK version + * - -h/--help: Show SDK help (optionally for a specific command) */ export class SdkInfoCommand extends BaseCLICommand { name = 'sdk_info'; - description = 'Get Fluent (ServiceNow SDK) information using native SDK flags (-v, -h, -d)'; + description = 'Get Fluent (ServiceNow SDK) information. Flags: -v (version), -h (help)'; arguments: CommandArgument[] = [ { name: 'flag', type: 'string', required: true, - description: 'SDK flag to execute (-v/--version, -h/--help, -d/--debug)', + description: 'SDK flag to execute: -v/--version (SDK version), -h/--help (SDK help)', }, { name: 'command', @@ -30,7 +32,7 @@ export class SdkInfoCommand extends BaseCLICommand { private getSdkCommand(): { command: string; baseArgs: string[]; workingDirectory: string } { // Use the project root path where @servicenow/sdk is installed const projectRoot = getProjectRootPath(); - + return { command: 'npx', baseArgs: ['now-sdk'], @@ -39,15 +41,35 @@ export class SdkInfoCommand extends BaseCLICommand { } async execute(args: Record): Promise { + // Validate base args (type checking and shell injection protection) this.validateArgs(args); const flag = args.flag as string; const command = args.command as string | undefined; - // Validate flag - const validFlags = ['-v', '--version', '-h', '--help', '-d', '--debug']; - if (!validFlags.includes(flag)) { - return CommandResultFactory.error(`Invalid flag '${flag}'. Valid flags: ${validFlags.join(', ')}`); + // Strict flag validation - must exactly match allowed patterns + const validFlagPattern = /^(-v|--version|-h|--help)$/; + if (!validFlagPattern.test(flag)) { + return CommandResultFactory.error( + `Invalid flag '${flag}'. Valid flags: -v/--version, -h/--help` + ); + } + + // Additional validation for the optional command argument + if (command !== undefined) { + // Command should only be used with help flag + if (flag !== '-h' && flag !== '--help') { + return CommandResultFactory.error( + 'The \'command\' argument is only valid with -h/--help flag' + ); + } + // Validate command contains only safe characters (alphanumeric, hyphen, underscore) + const safeCommandPattern = /^[a-zA-Z0-9_-]+$/; + if (!safeCommandPattern.test(command)) { + return CommandResultFactory.error( + `Invalid command name '${command}'. Command must contain only alphanumeric characters, hyphens, and underscores.` + ); + } } // Build command arguments for SDK execution @@ -63,7 +85,7 @@ export class SdkInfoCommand extends BaseCLICommand { sdkArgs.push('--help'); } } else { - // For version and debug: npx now-sdk + // For version: npx now-sdk sdkArgs.push(flag); } @@ -108,10 +130,6 @@ export class SdkInfoCommand extends BaseCLICommand { } return `ServiceNow SDK General Help:\n\n${trimmed}\n\nNote: Retrieved using 'npx now-sdk --help'`; - case '-d': - case '--debug': - return `ServiceNow SDK Debug Information:\n\n${trimmed}\n\nNote: Retrieved using 'npx now-sdk --debug'`; - default: return trimmed; } diff --git a/src/tools/resourceTools.ts b/src/tools/resourceTools.ts index 72d84d9..8e2b892 100644 --- a/src/tools/resourceTools.ts +++ b/src/tools/resourceTools.ts @@ -90,11 +90,56 @@ export abstract class BaseResourceCommand implements CLICommand { /** * Command for accessing API specifications + * When called without metadataType, lists all available metadata types */ export class GetApiSpecCommand extends BaseResourceCommand { name = 'get-api-spec'; - description = 'Fetches the Fluent API specification for a given ServiceNow metadata type (e.g., "business-rule", "acl", etc.).'; + description = 'Fetches the Fluent API specification for a given ServiceNow metadata type. Call without arguments to list all available metadata types.'; resourceType = ResourceType.SPEC; + + // Override arguments to make metadataType optional for listing + arguments: CommandArgument[] = [ + { + name: 'metadataType', + type: 'string', + required: false, // Optional - if not provided, lists all available types + description: 'ServiceNow metadata type (e.g., business-rule, script-include). Omit to list all available types.', + }, + ]; + + /** + * Override doExecute to add listing functionality when no metadataType is provided + */ + protected async doExecute(args: Record): Promise { + const metadataType = args.metadataType as string | undefined; + + // If no metadataType provided, list all available types + if (!metadataType) { + try { + const metadataTypes = await this.resourceLoader.getAvailableMetadataTypes(); + + if (metadataTypes.length === 0) { + return CommandResultFactory.error('No metadata types found.'); + } + + const output = `Available Fluent metadata types (${metadataTypes.length}):\n${metadataTypes.join('\n')}\n\nUse get-api-spec with a specific metadataType to fetch its API specification.`; + return CommandResultFactory.success(output); + } catch (error) { + logger.error('Error listing metadata types', CommandResultFactory.normalizeError(error)); + return CommandResultFactory.fromError(error); + } + } + + // Otherwise, fetch the specific API spec + return super.doExecute(args); + } + + /** + * Override validateArgs since metadataType is now optional + */ + protected validateArgs(_args: Record): void { + // No validation needed - metadataType is optional + } } /** @@ -155,47 +200,6 @@ export class GetInstructCommand extends BaseResourceCommand { resourceType = ResourceType.INSTRUCT; } -/** - * Command for listing available metadata types - */ -export class ListMetadataTypesCommand implements CLICommand { - name = 'list-metadata-types'; - description = 'List all available ServiceNow metadata types that currently supported by Fluent (ServiceNow SDK)'; - arguments: CommandArgument[] = []; - - private resourceLoader: ResourceLoader; - - constructor() { - this.resourceLoader = new ResourceLoader(); - } - - /** - * Resource commands don't use command processors - */ - getCommandProcessor(): undefined { - return undefined; - } - - /** - * Execute the command to list available metadata types - * @returns Command result with list of metadata types - */ - async execute(): Promise { - try { - const metadataTypes = await this.resourceLoader.getAvailableMetadataTypes(); - - const output = metadataTypes.length === 0 - ? 'No metadata types found.' - : `Available metadata types:\n${metadataTypes.join('\n')}`; - - return CommandResultFactory.success(output); - } catch (error) { - logger.error('Error listing metadata types', CommandResultFactory.normalizeError(error)); - return CommandResultFactory.fromError(error); - } - } -} - /** * Command for checking current authentication status * Returns the cached auth validation result from the session diff --git a/src/tools/toolsManager.ts b/src/tools/toolsManager.ts index 6a78406..45a4cea 100644 --- a/src/tools/toolsManager.ts +++ b/src/tools/toolsManager.ts @@ -7,7 +7,6 @@ import { GetApiSpecCommand, GetSnippetCommand, GetInstructCommand, - ListMetadataTypesCommand, CheckAuthStatusCommand } from './resourceTools.js'; import { CLIExecutor, CLICmdWriter, NodeProcessRunner, BaseCommandProcessor } from './cliCommandTools.js'; @@ -29,7 +28,7 @@ export class ToolsManager { constructor(mcpServer: McpServer) { this.mcpServer = mcpServer; this.commandRegistry = new CommandRegistry(); - + // Initialize the tools this.initializeTools(); } @@ -40,14 +39,14 @@ export class ToolsManager { private initializeTools(): void { // Register CLI commands const processRunner = new NodeProcessRunner(); - + // Create both types of command processors const cliExecutor = new CLIExecutor(processRunner); const cliCmdWriter = new CLICmdWriter(); // CLICmdWriter doesn't need processRunner // Store shared processors for later use (e.g., server-internal invocations) this.cliExecutor = cliExecutor; this.cliCmdWriter = cliCmdWriter; - + // Create commands with appropriate processors for each type // InitCommand will use CLICmdWriter, others will use CLIExecutor // Note: AuthCommand is not exposed to MCP clients - it's used internally for auto-auth validation @@ -58,7 +57,7 @@ export class ToolsManager { // Register each CLI command as an MCP tool this.registerToolFromCommand(command); }); - + // Register resource tools this.registerResourceTools(); } @@ -68,11 +67,6 @@ export class ToolsManager { */ private registerResourceTools(): void { try { - // Register metadata type listing tool - const listMetadataTypesCommand = new ListMetadataTypesCommand(); - this.commandRegistry.register(listMetadataTypesCommand); - this.registerToolFromCommand(listMetadataTypesCommand); - // Register API specification tool const getApiSpecCommand = new GetApiSpecCommand(); this.commandRegistry.register(getApiSpecCommand); @@ -101,14 +95,14 @@ export class ToolsManager { throw error; } } - + /** * Registers a command as an MCP tool * @param command The command to register */ private registerToolFromCommand(command: CLICommand): void { if (!this.mcpServer) return; - + // Convert command arguments to Zod schema const schema: Record = {}; @@ -142,7 +136,7 @@ export class ToolsManager { schema[arg.name] = zodType; } - + // Create schema for MCP - use raw schema object, let MCP handle the z.object() wrapping let inputSchema: any = undefined; if (Object.keys(schema).length > 0) { @@ -159,11 +153,30 @@ export class ToolsManager { inputSchema: inputSchema }, async (args: { [x: string]: any }, _extra: unknown) => { - const result = await command.execute(args); - return { - content: [{ type: 'text' as const, text: result.output }], - structuredContent: { success: result.success } - }; + try { + const result = await command.execute(args); + + // Format the output for display (with ✅/❌ prefixes) + const formattedOutput = this.formatResult({ + success: result.success, + output: result.output, + exitCode: result.exitCode, + error: result.error?.message + }); + + return { + content: [{ type: 'text' as const, text: formattedOutput }], + isError: !result.success + }; + } catch (error) { + // Handle exceptions from validateArgs() or command execution + const errorMessage = error instanceof Error ? error.message : String(error); + logger.error(`Tool '${command.name}' execution failed`, error instanceof Error ? error : new Error(errorMessage)); + return { + content: [{ type: 'text' as const, text: `❌ Error: ${errorMessage}` }], + isError: true + }; + } } ); } @@ -177,9 +190,8 @@ export class ToolsManager { if (result.success) { return `✅ Command executed successfully\n\nOutput:\n${result.output}`; } else { - return `❌ Command failed (exit code: ${result.exitCode})\n\nError:\n${ - result.error || 'Unknown error' - }\n\nOutput:\n${result.output}`; + return `❌ Command failed (exit code: ${result.exitCode})\n\nError:\n${result.error || 'Unknown error' + }\n\nOutput:\n${result.output}`; } } @@ -207,7 +219,7 @@ export class ToolsManager { getCommandRegistry(): CommandRegistry { return this.commandRegistry; } - + /** * Update the roots in CLI tools * @param roots Array of root URIs and optional names diff --git a/test/tools/newCommands.test.ts b/test/tools/newCommands.test.ts index df2d239..2ed8d2e 100644 --- a/test/tools/newCommands.test.ts +++ b/test/tools/newCommands.test.ts @@ -22,7 +22,14 @@ jest.mock('../../src/utils/logger.js', () => ({ // Mock the config module jest.mock('../../src/config.js', () => ({ - getProjectRootPath: jest.fn().mockReturnValue('/mock/project/root') + getProjectRootPath: jest.fn().mockReturnValue('/mock/project/root'), + getConfig: jest.fn().mockReturnValue({ + resourcePaths: { + spec: '/mock/spec', + snippet: '/mock/snippet', + instruct: '/mock/instruct' + } + }) })); // Mock the rootContext module @@ -50,7 +57,7 @@ jest.mock('../../src/utils/sessionManager.js', () => ({ describe('New SDK Commands', () => { let mockProcessor: { process: jest.Mock }; - + beforeEach(() => { jest.clearAllMocks(); mockProcessor = { @@ -65,30 +72,30 @@ describe('New SDK Commands', () => { describe('DownloadCommand', () => { test('should create DownloadCommand with correct metadata', () => { const command = new DownloadCommand(mockProcessor as any); - + expect(command.name).toBe('download_fluent_app'); expect(command.description).toBe('Download application metadata from instance, including metadata that not exist in local but deployed to instance'); expect(command.arguments).toHaveLength(4); - + // Check required directory argument const directoryArg = command.arguments.find(arg => arg.name === 'directory'); expect(directoryArg).toBeDefined(); expect(directoryArg?.required).toBe(true); expect(directoryArg?.type).toBe('string'); expect(directoryArg?.description).toBe('Path to expand application'); - + // Check optional source argument const sourceArg = command.arguments.find(arg => arg.name === 'source'); expect(sourceArg).toBeDefined(); expect(sourceArg?.required).toBe(false); expect(sourceArg?.type).toBe('string'); - + // Check optional incremental argument const incrementalArg = command.arguments.find(arg => arg.name === 'incremental'); expect(incrementalArg).toBeDefined(); expect(incrementalArg?.required).toBe(false); expect(incrementalArg?.type).toBe('boolean'); - + // Check optional debug argument const debugArg = command.arguments.find(arg => arg.name === 'debug'); expect(debugArg).toBeDefined(); @@ -98,9 +105,9 @@ describe('New SDK Commands', () => { test('should execute download command with required directory', async () => { const command = new DownloadCommand(mockProcessor as any); - + const result = await command.execute({ directory: 'my-app' }); - + expect(result.success).toBe(true); expect(result.output).toContain('Mock command output'); expect(mockProcessor.process).toHaveBeenCalledWith( @@ -113,14 +120,14 @@ describe('New SDK Commands', () => { test('should execute download command with all arguments', async () => { const command = new DownloadCommand(mockProcessor as any); - + const result = await command.execute({ directory: 'my-app', source: './src', incremental: true, debug: true }); - + expect(result.success).toBe(true); expect(mockProcessor.process).toHaveBeenCalledWith( 'npx', @@ -134,18 +141,18 @@ describe('New SDK Commands', () => { describe('CleanCommand', () => { test('should create CleanCommand with correct metadata', () => { const command = new CleanCommand(mockProcessor as any); - + expect(command.name).toBe('clean_fluent_app'); expect(command.description).toBe('Clean up output directory of a Fluent (ServiceNow SDK) application'); expect(command.arguments).toHaveLength(2); - + // Check optional source argument const sourceArg = command.arguments.find(arg => arg.name === 'source'); expect(sourceArg).toBeDefined(); expect(sourceArg?.required).toBe(false); expect(sourceArg?.type).toBe('string'); expect(sourceArg?.description).toBe('Path to the directory that contains package.json configuration'); - + // Check optional debug argument const debugArg = command.arguments.find(arg => arg.name === 'debug'); expect(debugArg).toBeDefined(); @@ -155,9 +162,9 @@ describe('New SDK Commands', () => { test('should execute clean command without arguments', async () => { const command = new CleanCommand(mockProcessor as any); - + const result = await command.execute({}); - + expect(result.success).toBe(true); expect(mockProcessor.process).toHaveBeenCalledWith( 'npx', @@ -169,12 +176,12 @@ describe('New SDK Commands', () => { test('should execute clean command with source and debug', async () => { const command = new CleanCommand(mockProcessor as any); - + const result = await command.execute({ source: 'src', debug: true }); - + expect(result.success).toBe(true); expect(mockProcessor.process).toHaveBeenCalledWith( 'npx', @@ -188,18 +195,18 @@ describe('New SDK Commands', () => { describe('PackCommand', () => { test('should create PackCommand with correct metadata', () => { const command = new PackCommand(mockProcessor as any); - + expect(command.name).toBe('pack_fluent_app'); expect(command.description).toBe('Zip built Fluent (ServiceNow SDK) application into installable artifact'); expect(command.arguments).toHaveLength(2); - + // Check optional source argument const sourceArg = command.arguments.find(arg => arg.name === 'source'); expect(sourceArg).toBeDefined(); expect(sourceArg?.required).toBe(false); expect(sourceArg?.type).toBe('string'); expect(sourceArg?.description).toBe('Path to the directory that contains package.json configuration'); - + // Check optional debug argument const debugArg = command.arguments.find(arg => arg.name === 'debug'); expect(debugArg).toBeDefined(); @@ -209,9 +216,9 @@ describe('New SDK Commands', () => { test('should execute pack command without arguments', async () => { const command = new PackCommand(mockProcessor as any); - + const result = await command.execute({}); - + expect(result.success).toBe(true); expect(mockProcessor.process).toHaveBeenCalledWith( 'npx', @@ -223,12 +230,12 @@ describe('New SDK Commands', () => { test('should execute pack command with source and debug', async () => { const command = new PackCommand(mockProcessor as any); - + const result = await command.execute({ source: './build', debug: true }); - + expect(result.success).toBe(true); expect(mockProcessor.process).toHaveBeenCalledWith( 'npx', @@ -243,9 +250,9 @@ describe('New SDK Commands', () => { test('should include new commands in CommandFactory.createCommands', () => { const mockExecutor = { process: jest.fn() }; const commands = CommandFactory.createCommands(mockExecutor as any); - + const commandNames = commands.map(cmd => cmd.name); - + expect(commandNames).toContain('download_fluent_app'); expect(commandNames).toContain('clean_fluent_app'); expect(commandNames).toContain('pack_fluent_app'); diff --git a/test/tools/resourceTools.test.ts b/test/tools/resourceTools.test.ts index ea27a2e..aec271a 100644 --- a/test/tools/resourceTools.test.ts +++ b/test/tools/resourceTools.test.ts @@ -7,7 +7,6 @@ import { GetApiSpecCommand, GetSnippetCommand, GetInstructCommand, - ListMetadataTypesCommand, } from "../../src/tools/resourceTools.js"; import { ResourceLoader, ResourceType } from "../../src/utils/resourceLoader.js"; import { getProjectRootPath } from "../../src/config.js"; @@ -30,20 +29,20 @@ describe("Resource Tool Commands", () => { // Test metadata type to use throughout tests const TEST_METADATA_TYPE = "business-rule"; const TEST_SNIPPET_ID = "0001"; - + // Set up ResourceLoader and mocks before each test let resourceLoader: ResourceLoader; const mockTypes = ["business-rule", "script-include", "client-script", "ui-action"]; const mockSpecContent = "Business Rule spec\nBusinessRule("; const mockSnippetContent = "Business Rule API example\nBusinessRule("; const mockInstructContent = "Instructions for Fluent Business Rule API"; - + beforeEach(() => { resourceLoader = new ResourceLoader(); - + // Reset all mocks jest.clearAllMocks(); - + // Setup default mocks for file operations (fs.promises.readdir as jest.Mock).mockImplementation((path: string) => { if (path.includes("instruct")) { @@ -62,7 +61,7 @@ describe("Resource Tool Commands", () => { } return Promise.resolve([]); }); - + (fs.promises.readFile as jest.Mock).mockImplementation((path: string) => { if (path.includes("spec")) { return Promise.resolve(mockSpecContent); @@ -73,130 +72,92 @@ describe("Resource Tool Commands", () => { } return Promise.resolve(""); }); - + (fs.existsSync as jest.Mock).mockImplementation((path: string) => { return path.includes("business-rule") || path.includes("0001"); }); }); - describe("ListMetadataTypesCommand", () => { - let command: ListMetadataTypesCommand; - - beforeEach(() => { - command = new ListMetadataTypesCommand(); - }); - - it("should return list of metadata types", async () => { - // Mock ResourceLoader to return our mock types - jest.spyOn(ResourceLoader.prototype, 'getAvailableMetadataTypes').mockResolvedValue(mockTypes); - - // Execute the command with our mocked ResourceLoader - const result = await command.execute(); - - // Verify the result contains the expected metadata types - expect(result.exitCode).toBe(0); - expect(result.success).toBe(true); - expect(result.output).toContain("Available metadata types:"); - - // Verify expected metadata types are in the output - mockTypes.forEach(type => { - expect(result.output).toContain(type); - }); - }); - - it("should handle errors when metadata types can't be retrieved", async () => { - // Create a spy on the getAvailableMetadataTypes method and force it to throw an error - jest.spyOn(ResourceLoader.prototype, "getAvailableMetadataTypes").mockImplementationOnce(() => { - throw new Error("Failed to get metadata types"); - }); - - const result = await command.execute(); - - expect(result.exitCode).toBe(1); - expect(result.success).toBe(false); - expect(result.output).toContain("Error:"); - expect(result.error).toBeDefined(); - }); - }); - describe("GetApiSpecCommand", () => { let command: GetApiSpecCommand; - + beforeEach(() => { command = new GetApiSpecCommand(); }); - + it("should return API specification for a valid metadata type", async () => { // Execute the command with a valid metadata type const result = await command.execute({ metadataType: TEST_METADATA_TYPE }); - + // Verify the result contains the expected content expect(result.exitCode).toBe(0); expect(result.success).toBe(true); expect(result.output).toBe(mockSpecContent); - + // Verify the content contains expected text for a business rule spec expect(result.output).toContain("Business Rule spec"); expect(result.output).toContain("BusinessRule("); }); - + it("should handle resource not found for non-existent metadata type", async () => { // Execute the command with a non-existent metadata type const result = await command.execute({ metadataType: "non-existent" }); - + expect(result.exitCode).toBe(1); expect(result.success).toBe(false); expect(result.output).toContain("Resource not found for metadata type: non-existent"); expect(result.error).toBeDefined(); }); - - it("should handle missing required arguments", async () => { - // Execute the command without required arguments + + it("should list available metadata types when no argument provided", async () => { + // Mock ResourceLoader to return our mock types + jest.spyOn(ResourceLoader.prototype, 'getAvailableMetadataTypes').mockResolvedValue(mockTypes); + + // Execute the command without arguments - should list available types const result = await command.execute({}); - - expect(result.exitCode).toBe(1); - expect(result.success).toBe(false); - expect(result.output).toContain("Error: Missing required argument: metadataType"); - expect(result.error).toBeDefined(); + + expect(result.exitCode).toBe(0); + expect(result.success).toBe(true); + expect(result.output).toContain("Available Fluent metadata types"); }); }); - + describe("GetSnippetCommand", () => { let command: GetSnippetCommand; - + beforeEach(() => { command = new GetSnippetCommand(); }); - + it("should return specific snippet when ID is provided", async () => { // Execute the command with a valid metadata type and snippet ID const result = await command.execute({ metadataType: TEST_METADATA_TYPE, id: TEST_SNIPPET_ID, }); - + // Verify the result contains the expected snippet content expect(result.exitCode).toBe(0); expect(result.success).toBe(true); expect(result.output).toBe(mockSnippetContent); - + // Verify the content contains expected text for a business rule snippet expect(result.output).toContain("Business Rule API example"); expect(result.output).toContain("BusinessRule("); }); - + it("should list available snippets when ID is not provided", async () => { // The mock will return snippet IDs: 0001, 0002, 0003 const mockSnippetIds = ["0001", "0002", "0003"]; - + // Execute the command without specifying a snippet ID const result = await command.execute({ metadataType: TEST_METADATA_TYPE }); - + // Verify the result contains the default snippet content and lists other available snippets expect(result.exitCode).toBe(0); expect(result.success).toBe(true); expect(result.output).toContain(mockSnippetContent); - + // Verify that additional snippets are listed (excluding the default one) expect(result.output).toContain("Additional snippets available:"); // Check for other snippet IDs (besides the default 0001) @@ -204,52 +165,52 @@ describe("Resource Tool Commands", () => { expect(result.output).toContain(id); }); }); - + it("should handle no snippets found for non-existent metadata type", async () => { // Execute the command with a non-existent metadata type const result = await command.execute({ metadataType: "non-existent" }); - + expect(result.exitCode).toBe(1); expect(result.success).toBe(false); expect(result.output).toContain("No snippets found for metadata type: non-existent"); expect(result.error).toBeDefined(); }); }); - + describe("GetInstructCommand", () => { let command: GetInstructCommand; - + beforeEach(() => { command = new GetInstructCommand(); }); - + it("should return instructions for a valid metadata type", async () => { // Execute the command with a valid metadata type const result = await command.execute({ metadataType: TEST_METADATA_TYPE }); - + // Verify the result contains the expected instruction content expect(result.exitCode).toBe(0); expect(result.success).toBe(true); expect(result.output).toBe(mockInstructContent); - + // Verify the content contains expected text for business rule instructions expect(result.output).toContain("Instructions for Fluent Business Rule API"); }); - + it("should handle resource not found for non-existent metadata type", async () => { // Execute the command with a non-existent metadata type const result = await command.execute({ metadataType: "non-existent" }); - + expect(result.exitCode).toBe(1); expect(result.success).toBe(false); expect(result.output).toContain("Resource not found for metadata type: non-existent"); expect(result.error).toBeDefined(); }); - + it("should handle missing required arguments", async () => { // Execute the command without required arguments const result = await command.execute({}); - + expect(result.exitCode).toBe(1); expect(result.success).toBe(false); expect(result.output).toContain("Error: Missing required argument: metadataType"); diff --git a/test/tools/sdkCommands.test.ts b/test/tools/sdkCommands.test.ts index 1982f48..02a85dc 100644 --- a/test/tools/sdkCommands.test.ts +++ b/test/tools/sdkCommands.test.ts @@ -137,34 +137,18 @@ describe('SDK Command Tools', () => { expect.any(String), // Working directory should be provided (project root path) undefined // stdinInput ); - + // Restore the original run method mockRunner.run = originalRun; }); - test('SdkInfoCommand should execute correctly with -d flag', async () => { + test('SdkInfoCommand should reject -lm flag (no longer supported)', async () => { const sdkInfoCommand = commands.find((cmd) => cmd.name === 'sdk_info'); expect(sdkInfoCommand).toBeDefined(); - // Set up mock response for debug flag - const originalRun = mockRunner.run; - mockRunner.run = jest.fn().mockImplementation(async (_command, args) => { - if (args.includes('--debug')) { - return { - stdout: 'Debug logs for ServiceNow SDK', - stderr: '', - exitCode: 0 - }; - } - return originalRun.call(mockRunner, _command, args); - }); - - const result = await sdkInfoCommand.execute({ flag: '-d' }); - expect(result.success).toBe(true); - expect(result.output).toContain('Debug logs for ServiceNow SDK'); - - // Restore the original run method - mockRunner.run = originalRun; + const result = await sdkInfoCommand.execute({ flag: '-lm' }); + expect(result.success).toBe(false); + expect(result.error?.message).toContain('Invalid flag'); }); test('SdkInfoCommand should handle invalid flags', async () => {