diff --git a/CHANGELOG.md b/CHANGELOG.md index a012cf6f..92fa8bcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `mcpc connect` now accepts an inline stdio command — quote the whole string (e.g., `mcpc connect "npx -y @modelcontextprotocol/server-filesystem /tmp"`) or use `--` after the session name (e.g., `mcpc connect @stdio -- node dist/stdio.js`). The session name is auto-generated from the binary basename with a numeric suffix (e.g., `@npx-1`, `@node-1`) when omitted; identical re-runs reuse the existing session. Auth flags (`--header`, `--profile`, `--no-profile`, `--x402`) are not allowed with inline commands. - New `tasks-result ` command that fetches the final `CallToolResult` payload of an async task via the MCP `tasks/result` method. Blocks until the task reaches a terminal state, then prints the payload using the same renderer as `tools-call` (`--json` returns the raw result). ## [0.2.5] - 2026-04-15 diff --git a/README.md b/README.md index 571ae2f2..ab04b30f 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,10 @@ mcpc --json @test tools-list # Use a local MCP server package (stdio) referenced from config file mcpc connect ./.vscode/mcp.json:filesystem @fs mcpc @fs tools-list + +# Or spawn a local stdio server inline (no config file needed) +mcpc connect "npx -y @modelcontextprotocol/server-filesystem ${PWD}" +mcpc @npx-1 tools-list ``` ## Usage @@ -178,6 +182,7 @@ The `connect`, `login`, and `logout` commands accept a `` argument in th - **Remote URL** (e.g. `mcp.apify.com` or `https://mcp.apify.com`) — scheme defaults to `https://` - **Config file entry** (e.g. `~/.vscode/mcp.json:filesystem`) — `file:entry-name` syntax +- **Inline stdio command** (e.g. `"npx -y @modelcontextprotocol/server-filesystem /tmp"`) — quote the whole command, or use `--` after the session name (e.g. `mcpc connect @fs -- node dist/stdio.js`). Your shell handles `${VAR}` expansion; `mcpc` does not substitute env vars in inline commands. ### MCP commands @@ -193,6 +198,11 @@ mcpc @apify tools-call search-apify-docs query:="What are Actors?" mcpc connect ~/.vscode/mcp.json:filesystem @fs mcpc @fs tools-list mcpc @fs tools-call list_directory path:=/ + +# Connect to a local stdio server inline (no config file) +mcpc connect "npx -y @modelcontextprotocol/server-filesystem ${PWD}" +mcpc connect @stdio -- node dist/stdio.js # explicit form via '--' +mcpc @npx-1 tools-list # auto-named from binary basename ``` See [MCP feature support](#mcp-feature-support) for details about all supported MCP features and commands. diff --git a/src/cli/commands/sessions.ts b/src/cli/commands/sessions.ts index a5ecada4..4cb6c3d4 100644 --- a/src/cli/commands/sessions.ts +++ b/src/cli/commands/sessions.ts @@ -7,11 +7,12 @@ import { OutputMode, isValidSessionName, generateSessionName, - normalizeServerUrl, validateProfileName, isProcessAlive, getServerHost, redactHeaders, + matchSessionByTarget, + pickAvailableSessionName, } from '../../lib/index.js'; import { DISCONNECTED_THRESHOLD_MS } from '../../lib/types.js'; import type { ServerConfig, ProxyConfig } from '../../lib/types.js'; @@ -90,57 +91,15 @@ async function checkPortAvailable(host: string, port: number): Promise * * @returns The matching session name (with @ prefix), or undefined if no match found */ -async function findMatchingSession( - parsed: { type: 'url'; url: string } | { type: 'config'; file: string; entry: string }, +export async function findMatchingSession( + parsed: + | { type: 'url'; url: string } + | { type: 'config'; file: string; entry: string } + | { type: 'command'; command: string; args: string[]; env?: Record }, options: { profile?: string; headers?: string[]; noProfile?: boolean } ): Promise { const storage = await loadSessions(); - const sessions = Object.values(storage.sessions); - - if (sessions.length === 0) return undefined; - - // Determine the effective profile name for comparison - const effectiveProfile = options.noProfile ? undefined : (options.profile ?? 'default'); - - for (const session of sessions) { - if (!session.server) continue; - - // Match server target - if (parsed.type === 'url') { - if (!session.server.url) continue; - // Compare normalized URLs - try { - const existingUrl = normalizeServerUrl(session.server.url); - const newUrl = normalizeServerUrl(parsed.url); - if (existingUrl !== newUrl) continue; - } catch { - continue; - } - } else { - // Config entry: match by command (stdio transport) - // Config entries produce stdio configs with command/args, so we can't easily - // compare them. Instead, just compare generated session names for config targets. - // This is handled by the caller (resolveSessionName) via name-based dedup. - continue; - } - - // Match profile - const sessionProfile = session.profileName ?? 'default'; - if (effectiveProfile !== sessionProfile) continue; - - // Match header keys (values are redacted, so we only compare key sets) - const existingHeaderKeys = Object.keys(session.server.headers || {}).sort(); - const newHeaderKeys = (options.headers || []) - .map((h) => h.split(':')[0]?.trim() || '') - .filter(Boolean) - .sort(); - if (existingHeaderKeys.join(',') !== newHeaderKeys.join(',')) continue; - - // Found a match - return session.name; - } - - return undefined; + return matchSessionByTarget(storage, parsed, options); } /** @@ -150,7 +109,10 @@ async function findMatchingSession( * @returns Session name with @ prefix */ export async function resolveSessionName( - parsed: { type: 'url'; url: string } | { type: 'config'; file: string; entry: string }, + parsed: + | { type: 'url'; url: string } + | { type: 'config'; file: string; entry: string } + | { type: 'command'; command: string; args: string[]; env?: Record }, options: { outputMode: OutputMode; profile?: string; @@ -167,29 +129,29 @@ export async function resolveSessionName( // Generate a new session name const candidateName = generateSessionName(parsed); - // Check if the candidate name is already taken by a different server + // For inline commands, always append a numeric suffix (starting at -1) since the binary + // basename is rarely as distinctive as a hostname or config entry. + // For URL/config targets, try the bare name first, then -2, -3, ... const storage = await loadSessions(); - if (!(candidateName in storage.sessions)) { + const picked = pickAvailableSessionName(storage, candidateName, parsed.type === 'command'); + if (picked) { if (options.outputMode === 'human') { - console.log(chalk.cyan(`Using session name: ${candidateName}`)); + console.log(chalk.cyan(`Using session name: ${picked}`)); } - return candidateName; + return picked; } - // Name is taken - try suffixed variants - for (let i = 2; i <= 99; i++) { - const suffixed = `${candidateName}-${i}`; - if (isValidSessionName(suffixed) && !(suffixed in storage.sessions)) { - if (options.outputMode === 'human') { - console.log(chalk.cyan(`Using session name: ${suffixed}`)); - } - return suffixed; - } + let targetDescription: string; + if (parsed.type === 'url') { + targetDescription = parsed.url; + } else if (parsed.type === 'config') { + targetDescription = `${parsed.file}:${parsed.entry}`; + } else { + targetDescription = [parsed.command, ...parsed.args].join(' '); } - throw new ClientError( `Cannot auto-generate session name: too many sessions for this server.\n` + - `Specify a name explicitly: mcpc connect ${parsed.type === 'url' ? parsed.url : `${parsed.file}:${parsed.entry}`} @my-session` + `Specify a name explicitly: mcpc connect ${targetDescription} @my-session` ); } @@ -214,6 +176,11 @@ export async function connectSession( insecure?: boolean; skipDetails?: boolean; quiet?: boolean; + /** + * Pre-built ServerConfig (for inline stdio commands). When provided, + * resolveTarget() is skipped and this config is used directly. + */ + inlineServerConfig?: ServerConfig; } ): Promise { // Validate session name @@ -281,8 +248,10 @@ export async function connectSession( } } - // Resolve target to transport config - const serverConfig = await resolveTarget(target, options); + // Resolve target to transport config (or use the pre-built inline config for stdio commands) + const serverConfig = options.inlineServerConfig + ? options.inlineServerConfig + : await resolveTarget(target, options); // Detect conflicting auth flags: --profile and --header "Authorization: ..." are mutually exclusive const hasExplicitAuthHeader = serverConfig.headers?.Authorization !== undefined; diff --git a/src/cli/index.ts b/src/cli/index.ts index 2d672617..a523b2b6 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -27,7 +27,7 @@ import * as tasks from './commands/tasks.js'; import * as grepCmd from './commands/grep.js'; import { handleX402Command } from './commands/x402.js'; import { clean } from './commands/clean.js'; -import type { OutputMode } from '../lib/index.js'; +import type { OutputMode, ServerConfig } from '../lib/index.js'; import { extractOptions, getVerboseFromEnv, @@ -133,6 +133,66 @@ function getOptionsFromCommand(command: Command): HandlerOptions { return options; } +/** + * Inline stdio command tokens captured from `mcpc connect [...] -- `. + * Set by extractInlineCommandTokens() before Commander parses argv, consumed by the + * connect action handler. Module-level storage avoids threading raw argv through Commander. + */ +let pendingInlineCommandTokens: string[] | undefined; + +/** + * If args contain `--` after the `connect` token, extract the trailing tokens as the inline + * stdio command and remove `--` and the trailing tokens from both `args` and `process.argv` + * so Commander never sees them. Stores the trailing tokens in `pendingInlineCommandTokens`. + * + * @returns The cleaned args array (with `--` and post-`--` tokens removed) + * @throws ClientError if `--` is used outside a `connect` command + */ +function extractInlineCommandTokens(args: string[]): string[] { + const dashIndex = args.indexOf('--'); + if (dashIndex < 0) return args; + + // `--` is only meaningful for the `connect` command. Find the first non-option token to + // verify we're in a connect invocation; reject otherwise so users get a clear error. + let connectIndex = -1; + for (let i = 0; i < dashIndex; i++) { + const arg = args[i]; + if (!arg) continue; + if (arg.startsWith('-')) { + if (optionTakesValue(arg) && !arg.includes('=') && i + 1 < dashIndex) { + i++; + } + continue; + } + if (arg === 'connect') { + connectIndex = i; + } + break; + } + if (connectIndex < 0) { + throw new ClientError( + `'--' separator is only supported with 'connect'.\n` + + `Example: mcpc connect @session -- npx -y @modelcontextprotocol/server-filesystem /` + ); + } + + const trailing = args.slice(dashIndex + 1); + if (trailing.length === 0) { + throw new ClientError( + `'--' must be followed by an inline stdio command.\n` + + `Example: mcpc connect @session -- npx -y @modelcontextprotocol/server-filesystem /` + ); + } + + pendingInlineCommandTokens = trailing; + + // Remove `--` and trailing tokens from process.argv so Commander never parses them. + // process.argv = [node, script, ...args], so dashIndex in args corresponds to dashIndex+2 in argv. + process.argv = process.argv.slice(0, dashIndex + 2); + + return args.slice(0, dashIndex); +} + /** * Format a JSON output help line with backtick-style Markdown formatting. * Optional schemaUrl adds a "Schema:" link for AI agents. @@ -146,7 +206,17 @@ function jsonHelp(description: string, shape?: string, schemaUrl?: string): stri const SCHEMA_BASE = 'https://modelcontextprotocol.io/specification/2025-11-25/schema'; async function main(): Promise { - const args = process.argv.slice(2); + let args = process.argv.slice(2); + + // Extract inline stdio command tokens (after `--`) for `mcpc connect`. This must happen + // before validateOptions(), which would otherwise reject `--` as an unknown option. + // Mutates process.argv to remove `--` and trailing tokens so Commander never sees them. + try { + args = extractInlineCommandTokens(args); + } catch (error) { + console.error(chalk.red(formatHumanError(error as Error, false))); + process.exit(1); + } // Set up cleanup handlers for graceful shutdown const handleExit = (): void => { @@ -449,24 +519,28 @@ ${chalk.bold('Server formats:')} mcp.apify.com Remote HTTP server (https:// added automatically) ~/.vscode/mcp.json:puppeteer Config file entry (file:entry) ~/.vscode/mcp.json Config file — connect all servers in the file + "npx -y @scope/server-foo" Inline stdio command (quote the whole string) + -- node dist/stdio.js Inline stdio command via '--' (everything after is argv) ${chalk.bold('Session name:')} If @session is omitted, a name is auto-generated from the server hostname - (e.g. mcp.apify.com → @apify) or config entry name. If a matching session - already exists (same server URL, OAuth profile, and HTTP header names), it - is reused (restarted if not live). Header values are not compared — they - are stored securely in OS keychain. + (e.g. mcp.apify.com → @apify), config entry name, or command basename + (e.g. "npx ..." → @npx-1, "node ..." → @node-1). If a matching session + already exists (same server URL, OAuth profile, and HTTP header names — or for + stdio commands, exact match on command + args + env), it is reused (restarted + if not live). Header values are not compared — they are stored securely in OS + keychain. When connecting all servers from a config file, @session cannot be specified. + +${chalk.bold('Inline stdio commands:')} + No \${VAR} substitution is applied to inline commands — your shell is the only + expansion layer. Use double quotes to interpolate (\`"node \${PWD}/dist/foo.js"\`) + or single quotes to pass the literal string. Auth flags (--header, --profile, + --no-profile, --x402) cannot be combined with an inline command. ${jsonHelp('`InitializeResult` object extended with `toolNames` and `_mcpc` metadata', '`{ protocolVersion, capabilities, serverInfo, instructions?, toolNames?, _mcpc }`', `${SCHEMA_BASE}#initializeresult`)}` ) .action(async (server, sessionName, opts, command) => { - if (!server) { - throw new ClientError( - 'Missing required argument: server\n\nExample: mcpc connect mcp.apify.com @myapp' - ); - } const globalOpts = getOptionsFromCommand(command); - const parsed = parseServerArg(server); // Extract --header from connect-specific opts const headers: string[] | undefined = opts.header @@ -475,11 +549,84 @@ ${jsonHelp('`InitializeResult` object extended with `toolNames` and `_mcpc` meta : [opts.header as string] : undefined; - if (!parsed) { - throw new ClientError( - `Invalid server: "${server}"\n\n` + - `Expected a URL (e.g. mcp.apify.com) or a config file entry (e.g. ~/.vscode/mcp.json:filesystem)` - ); + // Inline stdio command via `--` separator: tokens were extracted before Commander ran. + // The `server` positional is forbidden in this form; if it looks like @session, shift it. + let parsed: ReturnType; + if (pendingInlineCommandTokens) { + if (server) { + if (server.startsWith('@') && !sessionName) { + // mcpc connect @stdio -- node dist/foo.js + sessionName = server; + server = undefined; + } else { + throw new ClientError( + `Cannot combine a server argument with '--'.\n` + + `Use either:\n` + + ` mcpc connect "${server}" [@session] (heuristic form)\n` + + ` mcpc connect [@session] -- ('--' form, no server arg)` + ); + } + } + const [cmd, ...rest] = pendingInlineCommandTokens; + if (!cmd) { + throw new ClientError(`'--' must be followed by an inline stdio command.`); + } + parsed = { type: 'command', command: cmd, args: rest }; + } else { + if (!server) { + throw new ClientError( + 'Missing required argument: server\n\nExample: mcpc connect mcp.apify.com @myapp' + ); + } + parsed = parseServerArg(server); + if (!parsed) { + throw new ClientError( + `Invalid server: "${server}"\n\n` + + `Expected a URL (e.g. mcp.apify.com), a config file entry (e.g. ~/.vscode/mcp.json:filesystem), or an inline stdio command (e.g. "npx -y @modelcontextprotocol/server-filesystem /").` + ); + } + } + + // Inline stdio command: validate incompatible flags and route via inlineServerConfig + if (parsed.type === 'command') { + const incompatibleFlags: string[] = []; + if (headers && headers.length > 0) incompatibleFlags.push('--header'); + if (globalOpts.profile) incompatibleFlags.push('--profile'); + if (globalOpts.noProfile) incompatibleFlags.push('--no-profile'); + if (globalOpts.x402) incompatibleFlags.push('--x402'); + if (incompatibleFlags.length > 0) { + throw new ClientError( + `${incompatibleFlags.join(', ')} cannot be combined with an inline stdio command.\n` + + `These flags only apply to HTTP servers.` + ); + } + + // Auto-generate session name if not provided + if (!sessionName) { + sessionName = await sessions.resolveSessionName(parsed, { + outputMode: globalOpts.outputMode, + }); + } + + const inlineServerConfig: ServerConfig = { + command: parsed.command, + args: parsed.args, + }; + // Inform the user what binary is being launched (security-relevant — see issue #163) + if (globalOpts.outputMode === 'human') { + const printable = [parsed.command, ...parsed.args] + .map((t) => (/\s/.test(t) ? `"${t}"` : t)) + .join(' '); + console.error(chalk.dim(`Launching: ${printable}`)); + } + await sessions.connectSession(parsed.command, sessionName, { + ...globalOpts, + inlineServerConfig, + proxy: opts.proxy, + proxyBearerToken: opts.proxyBearerToken, + ...(globalOpts.insecure && { insecure: true }), + }); + return; } // Config file without :entry — connect all servers from the file diff --git a/src/cli/parser.ts b/src/cli/parser.ts index 23064502..4cd59f3e 100644 --- a/src/cli/parser.ts +++ b/src/cli/parser.ts @@ -2,7 +2,7 @@ * Command-line argument parsing utilities * Pure functions with no external dependencies for easy testing */ -import { ClientError } from '../lib/index.js'; +import { ClientError, shellSplit } from '../lib/index.js'; /** * Check if an environment variable is set to a truthy value @@ -348,7 +348,7 @@ function looksLikeFilePath(s: string): boolean { } /** - * Parse a server argument into a URL or config file entry. + * Parse a server argument into a URL, config file entry, or inline stdio command. * * 1. URL: arg (as-is, or prefixed with https:// or http://) is a valid URL with a non-empty host. * Args that start with a path character (/, ~, .) skip the prefix check to avoid false positives @@ -356,7 +356,10 @@ function looksLikeFilePath(s: string): boolean { * 2. If arg contains "://" but failed URL validation above → null (invalid full-URL syntax). * 3. Config entry: colon present, entry non-empty, AND left side looks like a file path. * Windows drive-letter paths (C:\...) use lastIndexOf(':') so the drive colon is skipped. - * 4. Otherwise: returns null (caller should report an error) + * 4. Bare config file path (no :entry suffix) — connect all servers from the file. + * 4a. Inline stdio command — arg contains whitespace and isn't a URL/config-file path. + * The string is shell-split into [command, ...args]. (NEW) + * 5. Otherwise: returns null (caller should report an error) */ export function parseServerArg( arg: string @@ -364,6 +367,7 @@ export function parseServerArg( | { type: 'url'; url: string } | { type: 'config'; file: string; entry: string } | { type: 'config-file'; file: string } + | { type: 'command'; command: string; args: string[] } | null { // Step 1a: try arg as-is (covers full URLs like https://... or ftp://...) if (isValidUrlWithHost(arg)) { @@ -380,11 +384,13 @@ export function parseServerArg( // Skip if arg starts with a path character — those are file paths, not hostnames. // Skip if arg ends with a config file extension (e.g., config.json) — clearly a file, not a hostname. // Skip if arg ends with ':' — dangling colon is not a valid hostname. + // Skip if arg contains whitespace — that's clearly an inline command, not a hostname. const isWindowsDrive = /^[A-Za-z]:[/\\]/.test(arg); const startsWithPathChar = arg.startsWith('/') || arg.startsWith('~') || arg.startsWith('.') || isWindowsDrive; const hasConfigExtension = /\.(json|yaml|yml)$/i.test(arg); - if (!startsWithPathChar && !hasConfigExtension && !arg.endsWith(':')) { + const hasWhitespace = /\s/.test(arg); + if (!startsWithPathChar && !hasConfigExtension && !arg.endsWith(':') && !hasWhitespace) { if (isValidUrlWithHost('https://' + arg)) { return { type: 'url', url: arg }; } @@ -405,11 +411,28 @@ export function parseServerArg( } // Step 4: bare config file path (no :entry suffix) — connect all servers from the file. - // Matches if the entire arg looks like a file path (e.g., ~/.vscode/mcp.json, ./config.json) - if (looksLikeFilePath(arg)) { + // Matches if the entire arg looks like a file path (e.g., ~/.vscode/mcp.json, ./config.json). + // Strings containing whitespace are only treated as config files when they end in a known + // extension (.json/.yaml/.yml) — otherwise they're far more likely to be inline stdio commands + // (e.g. "npx -y @scope/pkg /tmp" contains a slash but isn't a file path). + if (looksLikeFilePath(arg) && (!hasWhitespace || hasConfigExtension)) { return { type: 'config-file', file: arg }; } + // Step 4a: inline stdio command — anything containing whitespace that wasn't a file or URL. + // shellSplit throws ClientError for unbalanced quotes / trailing backslashes. + if (hasWhitespace) { + const tokens = shellSplit(arg); + if (tokens.length === 0) { + return null; + } + const [command, ...rest] = tokens; + if (!command) { + return null; + } + return { type: 'command', command, args: rest }; + } + // Step 5: unrecognised return null; } diff --git a/src/lib/index.ts b/src/lib/index.ts index 42abaa53..cbda1bbf 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -20,3 +20,6 @@ export * from './file-logger.js'; // Export cleanup utilities export * from './cleanup.js'; + +// Export session-matching helpers +export * from './session-matching.js'; diff --git a/src/lib/session-matching.ts b/src/lib/session-matching.ts new file mode 100644 index 00000000..b0666a1f --- /dev/null +++ b/src/lib/session-matching.ts @@ -0,0 +1,126 @@ +/** + * Pure session-matching helpers, extracted from CLI command handlers so they + * can be unit-tested without pulling in chalk or other CLI-only deps. + */ + +import type { SessionsStorage } from './types.js'; +import { isValidSessionName, normalizeServerUrl } from './utils.js'; + +/** + * Parsed-target shape consumed by matchSessionByTarget. Mirrors the union + * accepted by parseServerArg + the inline-command branch from the CLI. + */ +export type ParsedTarget = + | { type: 'url'; url: string } + | { type: 'config'; file: string; entry: string } + | { type: 'command'; command: string; args: string[]; env?: Record }; + +/** + * Find an existing session in `storage` that matches the given parsed target and + * authentication settings. Pure function — no I/O, suitable for unit tests. + * + * Matching rules: + * - URL targets: normalized URL equality; profile and header-key set equality + * - Config targets: not matched here (caller falls back to name-based dedup) + * - Command targets: exact equality on command + args + env (after substitution) + * + * @returns The matching session name (e.g. "@apify"), or undefined if no match + */ +export function matchSessionByTarget( + storage: SessionsStorage, + parsed: ParsedTarget, + options: { profile?: string; headers?: string[]; noProfile?: boolean } +): string | undefined { + const sessions = Object.values(storage.sessions); + if (sessions.length === 0) return undefined; + + const effectiveProfile = options.noProfile ? undefined : (options.profile ?? 'default'); + + for (const session of sessions) { + if (!session.server) continue; + + if (parsed.type === 'url') { + if (!session.server.url) continue; + try { + const existingUrl = normalizeServerUrl(session.server.url); + const newUrl = normalizeServerUrl(parsed.url); + if (existingUrl !== newUrl) continue; + } catch { + continue; + } + } else if (parsed.type === 'command') { + if (!session.server.command) continue; + if (session.server.command !== parsed.command) continue; + const existingArgs = session.server.args || []; + if (existingArgs.length !== parsed.args.length) continue; + let argsMatch = true; + for (let i = 0; i < existingArgs.length; i++) { + if (existingArgs[i] !== parsed.args[i]) { + argsMatch = false; + break; + } + } + if (!argsMatch) continue; + const existingEnv = session.server.env || {}; + const newEnv = parsed.env || {}; + const existingEnvKeys = Object.keys(existingEnv).sort(); + const newEnvKeys = Object.keys(newEnv).sort(); + if (existingEnvKeys.length !== newEnvKeys.length) continue; + let envMatch = true; + for (let i = 0; i < existingEnvKeys.length; i++) { + const k = existingEnvKeys[i] as string; + if (k !== newEnvKeys[i] || existingEnv[k] !== newEnv[k]) { + envMatch = false; + break; + } + } + if (!envMatch) continue; + } else { + // Config entry: caller handles via name-based dedup. + continue; + } + + const sessionProfile = session.profileName ?? 'default'; + if (effectiveProfile !== sessionProfile) continue; + + const existingHeaderKeys = Object.keys(session.server.headers || {}).sort(); + const newHeaderKeys = (options.headers || []) + .map((h) => h.split(':')[0]?.trim() || '') + .filter(Boolean) + .sort(); + if (existingHeaderKeys.join(',') !== newHeaderKeys.join(',')) continue; + + return session.name; + } + + return undefined; +} + +/** + * Pick an available session name based on a candidate. + * + * @param storage Loaded sessions storage to check for collisions. + * @param candidate Base session name (e.g. "@npx", "@apify"). + * @param alwaysSuffix When true, always append "-N" starting at 1 (used for inline + * stdio commands where the binary basename is rarely distinctive). + * When false, try the bare candidate first, then "-2", "-3", ... + * @returns The first available session name, or undefined if all 99 suffixes are taken. + */ +export function pickAvailableSessionName( + storage: SessionsStorage, + candidate: string, + alwaysSuffix: boolean +): string | undefined { + if (!alwaysSuffix && !(candidate in storage.sessions)) { + return candidate; + } + + const startIndex = alwaysSuffix ? 1 : 2; + for (let i = startIndex; i <= 99; i++) { + const suffixed = `${candidate}-${i}`; + if (isValidSessionName(suffixed) && !(suffixed in storage.sessions)) { + return suffixed; + } + } + return undefined; +} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 46f318e7..c3fc00e9 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -219,6 +219,115 @@ export function isValidSessionName(name: string): boolean { return /^@[a-zA-Z0-9_-]{1,64}$/.test(name); } +/** + * Split a shell-style command string into argv tokens. + * + * Supports: + * - Whitespace as token separator (spaces and tabs, consecutive runs collapsed) + * - Single quotes (literal, no escapes inside) + * - Double quotes (allow `\"`, `\\`, `\$`, `\`` escapes) + * - Backslash escapes outside quotes (`\` → ``) + * + * Does NOT perform variable expansion (`${VAR}` is preserved verbatim). + * + * @throws ClientError if quoting is unbalanced or a trailing backslash is present. + */ +export function shellSplit(input: string): string[] { + const tokens: string[] = []; + let current = ''; + let hasToken = false; // distinguishes empty quoted token "" from no token + let i = 0; + const n = input.length; + + while (i < n) { + const ch = input[i] as string; + + if (ch === ' ' || ch === '\t' || ch === '\n') { + if (hasToken) { + tokens.push(current); + current = ''; + hasToken = false; + } + i++; + continue; + } + + if (ch === "'") { + // Single-quoted: everything literal until next single quote + const close = input.indexOf("'", i + 1); + if (close === -1) { + throw new ClientError( + `Unbalanced single quote in command string: ${input}\n` + + `Tip: each opening quote must have a matching closing quote.` + ); + } + current += input.slice(i + 1, close); + hasToken = true; + i = close + 1; + continue; + } + + if (ch === '"') { + // Double-quoted: handle a small set of backslash escapes + i++; + let closed = false; + while (i < n) { + const c = input[i] as string; + if (c === '"') { + closed = true; + i++; + break; + } + if (c === '\\' && i + 1 < n) { + const next = input[i + 1] as string; + if (next === '"' || next === '\\' || next === '$' || next === '`') { + current += next; + i += 2; + continue; + } + // Unrecognised escape: keep the backslash literal (matches POSIX shell) + current += c; + i++; + continue; + } + current += c; + i++; + } + if (!closed) { + throw new ClientError( + `Unbalanced double quote in command string: ${input}\n` + + `Tip: each opening quote must have a matching closing quote.` + ); + } + hasToken = true; + continue; + } + + if (ch === '\\') { + if (i + 1 >= n) { + throw new ClientError( + `Trailing backslash in command string: ${input}\n` + + `Tip: escape the backslash itself (\\\\) if you meant a literal backslash.` + ); + } + current += input[i + 1] as string; + hasToken = true; + i += 2; + continue; + } + + current += ch; + hasToken = true; + i++; + } + + if (hasToken) { + tokens.push(current); + } + + return tokens; +} + /** Common hostname prefixes to strip when generating session names */ const COMMON_HOST_PREFIXES = ['mcp.', 'api.', 'www.']; @@ -248,16 +357,34 @@ function sanitizeSessionName(raw: string): string { * For config entries: uses the entry name directly (sanitized). * Example: ~/.vscode/mcp.json:filesystem → filesystem * + * For inline commands: uses the basename of the command binary, with common script + * extensions stripped. Caller is expected to always append a `-N` suffix because the + * binary name is rarely as distinctive as a hostname or config entry. + * Examples: npx -y ... → @npx (caller adds -1), node dist/stdio.js → @node, + * /usr/local/bin/python → @python, server.py → @server, ./my-mcp-server → @my-mcp-server + * * @returns Session name with @ prefix (e.g., @apify) */ export function generateSessionName( - parsed: { type: 'url'; url: string } | { type: 'config'; file: string; entry: string } + parsed: + | { type: 'url'; url: string } + | { type: 'config'; file: string; entry: string } + | { type: 'command'; command: string; args: string[] } ): string { if (parsed.type === 'config') { const name = sanitizeSessionName(parsed.entry); return `@${name || 'session'}`; } + if (parsed.type === 'command') { + // basename: strip directory, then strip common script extensions + const slashIdx = Math.max(parsed.command.lastIndexOf('/'), parsed.command.lastIndexOf('\\')); + let base = slashIdx >= 0 ? parsed.command.slice(slashIdx + 1) : parsed.command; + base = base.replace(/\.(js|mjs|cjs|ts|py|sh|exe|bat|cmd)$/i, ''); + const sanitized = sanitizeSessionName(base); + return `@${sanitized || 'session'}`; + } + // URL case: parse and extract hostname const url = new URL(normalizeServerUrl(parsed.url)); let hostname = url.hostname.toLowerCase(); diff --git a/test/e2e/suites/stdio/inline-command.test.sh b/test/e2e/suites/stdio/inline-command.test.sh new file mode 100755 index 00000000..09aa495f --- /dev/null +++ b/test/e2e/suites/stdio/inline-command.test.sh @@ -0,0 +1,130 @@ +#!/bin/bash +# Test: Inline stdio command for `mcpc connect` (issue #163) +# +# Covers both surface forms (heuristic quoted string + `--` separator), +# auto-generated session names, session reuse, and flag-validation errors. +# +# The flag-validation tests run first because they don't actually spawn any +# child process (the connect action rejects the flags before launching). + +source "$(dirname "$0")/../../lib/framework.sh" +test_init "stdio/inline-command" + +SHORT="$_TEST_SHORT_ID" +NATIVE_TMP="$(to_native_path "$TEST_TMP")" + +# A bogus path is fine for flag-validation tests since the connect action rejects +# the incompatible flag before attempting to spawn anything. +INLINE_CMD="echo unused-but-must-have-spaces" + +# ============================================================================= +# Flag-validation errors (no MCP connection, no spawning) +# ============================================================================= + +# Test 1: --profile cannot be combined with inline command +test_case "--profile rejected with inline command" +run_mcpc connect "$INLINE_CMD" "@e-${SHORT}-bad" --profile default +assert_failure +assert_contains "$STDERR" "--profile" +test_pass + +# Test 2: --header cannot be combined with inline command +test_case "--header rejected with inline command" +run_mcpc connect "$INLINE_CMD" "@e-${SHORT}-bad" --header "X-Test: 1" +assert_failure +assert_contains "$STDERR" "--header" +test_pass + +# Test 3: --x402 cannot be combined with inline command +test_case "--x402 rejected with inline command" +run_mcpc connect "$INLINE_CMD" "@e-${SHORT}-bad" --x402 +assert_failure +assert_contains "$STDERR" "--x402" +test_pass + +# Test 4: combining a server arg with `--` is rejected +test_case "combining server arg with -- is rejected" +run_mcpc connect mcp.apify.com -- node dist/foo.js +assert_failure +assert_contains "$STDERR" "Cannot combine" +test_pass + +# Test 5: `--` with no trailing tokens is rejected +test_case "-- with no command is rejected" +run_mcpc connect "@e-${SHORT}-bad" -- +assert_failure +assert_contains "$STDERR" "must be followed" +test_pass + +# Test 6: `--` outside connect is rejected +test_case "-- outside connect is rejected" +run_mcpc tools-list -- something +assert_failure +assert_contains "$STDERR" "only supported with 'connect'" +test_pass + +# ============================================================================= +# Real stdio connect via inline command (requires npx + network) +# ============================================================================= + +# Test 7: heuristic form with explicit @session +SESSION1="@e-${SHORT}-h1" +test_case "heuristic form: connect with explicit @session" +run_mcpc connect "npx -y @modelcontextprotocol/server-filesystem $NATIVE_TMP" "$SESSION1" +assert_success +_SESSIONS_CREATED+=("$SESSION1") +test_pass + +# Test 8: tools-list works via heuristic-form session +test_case "heuristic form: tools-list works" +run_mcpc "$SESSION1" tools-list +assert_success +assert_contains "$STDOUT" "read_file" +test_pass + +# Test 9: session shows stdio transport (command field present, no url) +test_case "heuristic form: session shows stdio transport" +run_mcpc --json +command_field=$(echo "$STDOUT" | jq -r ".sessions[] | select(.name == \"$SESSION1\") | .server.command") +assert_eq "$command_field" "npx" "command field should be 'npx' for inline stdio session" +url_field=$(echo "$STDOUT" | jq -r ".sessions[] | select(.name == \"$SESSION1\") | .server.url // empty") +assert_eq "$url_field" "" "url field should be empty for stdio session" +test_pass + +# Test 10: re-running identical heuristic connect reuses the session ("already active") +test_case "heuristic form: identical re-run reuses session" +run_mcpc connect "npx -y @modelcontextprotocol/server-filesystem $NATIVE_TMP" "$SESSION1" +assert_success +assert_contains "$STDOUT" "already active" +test_pass + +# Test 11: close the heuristic-form session +test_case "close heuristic-form session" +run_mcpc "$SESSION1" close +assert_success +_SESSIONS_CREATED=("${_SESSIONS_CREATED[@]/$SESSION1}") +test_pass + +# Test 12: `--` form with explicit @session +SESSION2="@e-${SHORT}-d1" +test_case "-- form: connect with explicit @session" +run_mcpc connect "$SESSION2" -- npx -y @modelcontextprotocol/server-filesystem "$NATIVE_TMP" +assert_success +_SESSIONS_CREATED+=("$SESSION2") +test_pass + +# Test 13: tools-list via `--`-form session +test_case "-- form: tools-list works" +run_mcpc "$SESSION2" tools-list +assert_success +assert_contains "$STDOUT" "read_file" +test_pass + +# Test 14: close the `--`-form session +test_case "close -- form session" +run_mcpc "$SESSION2" close +assert_success +_SESSIONS_CREATED=("${_SESSIONS_CREATED[@]/$SESSION2}") +test_pass + +test_done diff --git a/test/unit/cli/index.test.ts b/test/unit/cli/index.test.ts index 50a7b83c..e3bef4ec 100644 --- a/test/unit/cli/index.test.ts +++ b/test/unit/cli/index.test.ts @@ -207,6 +207,101 @@ describe('parseServerArg', () => { expect(parseServerArg('example.com')).toEqual({ type: 'url', url: 'example.com' }); expect(parseServerArg('mcp.apify.com')).toEqual({ type: 'url', url: 'mcp.apify.com' }); }); + + describe('inline stdio command', () => { + it('should parse a simple command with args', () => { + expect(parseServerArg('npx -y foo')).toEqual({ + type: 'command', + command: 'npx', + args: ['-y', 'foo'], + }); + }); + + it('should parse "node dist/stdio.js" as inline command', () => { + expect(parseServerArg('node dist/stdio.js')).toEqual({ + type: 'command', + command: 'node', + args: ['dist/stdio.js'], + }); + }); + + it('should parse a real-world npx command', () => { + expect(parseServerArg('npx -y @modelcontextprotocol/server-filesystem /')).toEqual({ + type: 'command', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-filesystem', '/'], + }); + }); + + it('should parse a uvx command with --flag=value', () => { + expect(parseServerArg('uvx mcp-server-time --local-timezone=Europe/Prague')).toEqual({ + type: 'command', + command: 'uvx', + args: ['mcp-server-time', '--local-timezone=Europe/Prague'], + }); + }); + + it('should preserve single-quoted tokens', () => { + expect(parseServerArg("python -c 'import x; x.run()'")).toEqual({ + type: 'command', + command: 'python', + args: ['-c', 'import x; x.run()'], + }); + }); + + it('should preserve double-quoted tokens with spaces', () => { + expect(parseServerArg('node "my server.js"')).toEqual({ + type: 'command', + command: 'node', + args: ['my server.js'], + }); + }); + + it('should preserve ${VAR}-looking literals (no expansion)', () => { + expect(parseServerArg("node 'dist/foo.js' '${PWD}/data'")).toEqual({ + type: 'command', + command: 'node', + args: ['dist/foo.js', '${PWD}/data'], + }); + }); + + it('should throw on unbalanced double quote', () => { + expect(() => parseServerArg('node "unclosed')).toThrow(/Unbalanced double quote/); + }); + + it('should throw on unbalanced single quote', () => { + expect(() => parseServerArg("node 'unclosed")).toThrow(/Unbalanced single quote/); + }); + + it('should NOT parse a single-word non-hostname as inline command', () => { + // No whitespace → falls through to URL step, which succeeds for any non-special token. + // Single-word stdio binaries require the `--` form (handled by the CLI, not parseServerArg). + expect(parseServerArg('mcp-fs')).toEqual({ type: 'url', url: 'mcp-fs' }); + }); + + it('should prefer config-file branch for paths-with-spaces ending in .json', () => { + // Config file extension wins over whitespace heuristic. + expect(parseServerArg('./path with space.json')).toEqual({ + type: 'config-file', + file: './path with space.json', + }); + }); + + it('should prefer config-file branch for absolute paths-with-spaces', () => { + // Path-character heuristic wins over whitespace heuristic. + expect(parseServerArg('./my dir/server.json')).toEqual({ + type: 'config-file', + file: './my dir/server.json', + }); + }); + + it('should not parse URL-with-port-and-path as command (no whitespace)', () => { + expect(parseServerArg('mcp.apify.com:8000/v1')).toEqual({ + type: 'url', + url: 'mcp.apify.com:8000/v1', + }); + }); + }); }); describe('extractOptions', () => { diff --git a/test/unit/lib/session-matching.test.ts b/test/unit/lib/session-matching.test.ts new file mode 100644 index 00000000..bc84af62 --- /dev/null +++ b/test/unit/lib/session-matching.test.ts @@ -0,0 +1,420 @@ +/** + * Unit tests for matchSessionByTarget (pure session-reuse logic). + * + * Covers the inline-stdio-command branch: exact match on command + args + env. + * The function is pure (no I/O), so we just feed it controlled SessionsStorage objects. + */ + +import { + matchSessionByTarget, + pickAvailableSessionName, +} from '../../../src/lib/session-matching.js'; +import type { SessionData, SessionsStorage } from '../../../src/lib/types.js'; + +function makeStorage(sessions: Record): SessionsStorage { + return { sessions }; +} + +function makeStdioSession( + name: string, + command: string, + args: string[], + env?: Record +): SessionData { + return { + name, + server: { + command, + args, + ...(env && { env }), + }, + createdAt: new Date().toISOString(), + }; +} + +describe('matchSessionByTarget — inline command targets', () => { + it('returns existing session when command + args match exactly', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'foo'] }, + {} + ); + expect(result).toBe('@npx-1'); + }); + + it('returns undefined when command differs', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'node', args: ['-y', 'foo'] }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('returns undefined when one arg differs', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'bar'] }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('returns undefined when arg count differs', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'foo', 'extra'] }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('returns undefined when arg order differs', () => { + const storage = makeStorage({ + '@node-1': makeStdioSession('@node-1', 'node', ['a', 'b']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'node', args: ['b', 'a'] }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('matches sessions with identical env vars', () => { + const storage = makeStorage({ + '@node-1': makeStdioSession('@node-1', 'node', ['dist/foo.js'], { API_KEY: 'secret' }), + }); + + const result = matchSessionByTarget( + storage, + { + type: 'command', + command: 'node', + args: ['dist/foo.js'], + env: { API_KEY: 'secret' }, + }, + {} + ); + expect(result).toBe('@node-1'); + }); + + it('returns undefined when env value differs', () => { + const storage = makeStorage({ + '@node-1': makeStdioSession('@node-1', 'node', ['dist/foo.js'], { API_KEY: 'secret' }), + }); + + const result = matchSessionByTarget( + storage, + { + type: 'command', + command: 'node', + args: ['dist/foo.js'], + env: { API_KEY: 'different' }, + }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('returns undefined when env keys differ', () => { + const storage = makeStorage({ + '@node-1': makeStdioSession('@node-1', 'node', ['dist/foo.js'], { API_KEY: 'secret' }), + }); + + const result = matchSessionByTarget( + storage, + { + type: 'command', + command: 'node', + args: ['dist/foo.js'], + env: { OTHER_KEY: 'secret' }, + }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('treats absent env as empty env (matches stored session with no env)', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'foo'] }, + {} + ); + expect(result).toBe('@npx-1'); + }); + + it('does not match when existing session is URL-based', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + createdAt: new Date().toISOString(), + }, + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'foo'] }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('does not match URL parsed when only stdio sessions exist', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + }); + + const result = matchSessionByTarget(storage, { type: 'url', url: 'https://mcp.apify.com' }, {}); + expect(result).toBeUndefined(); + }); + + it('picks the matching session when multiple stdio sessions share the binary', () => { + const storage = makeStorage({ + '@npx-1': makeStdioSession('@npx-1', 'npx', ['-y', 'foo']), + '@npx-2': makeStdioSession('@npx-2', 'npx', ['-y', 'bar']), + }); + + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'bar'] }, + {} + ); + expect(result).toBe('@npx-2'); + }); + + it('returns undefined when storage is empty', () => { + const storage = makeStorage({}); + const result = matchSessionByTarget( + storage, + { type: 'command', command: 'npx', args: ['-y', 'foo'] }, + {} + ); + expect(result).toBeUndefined(); + }); + + it('returns undefined for config-entry parsed targets (caller falls back to name dedup)', () => { + const storage = makeStorage({ + '@filesystem': makeStdioSession('@filesystem', 'npx', [ + '-y', + '@modelcontextprotocol/server-filesystem', + '/tmp', + ]), + }); + + const result = matchSessionByTarget( + storage, + { type: 'config', file: './mcp.json', entry: 'filesystem' }, + {} + ); + expect(result).toBeUndefined(); + }); +}); + +describe('matchSessionByTarget — URL targets (regression)', () => { + it('matches normalized URLs', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + createdAt: new Date().toISOString(), + }, + }); + + expect(matchSessionByTarget(storage, { type: 'url', url: 'https://mcp.apify.com' }, {})).toBe( + '@apify' + ); + expect(matchSessionByTarget(storage, { type: 'url', url: 'mcp.apify.com' }, {})).toBe('@apify'); + }); + + it('does not match different URLs', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + createdAt: new Date().toISOString(), + }, + }); + expect(matchSessionByTarget(storage, { type: 'url', url: 'https://example.com' }, {})).toBe( + undefined + ); + }); + + it('matches when profile is "default" by default', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + profileName: 'default', + createdAt: new Date().toISOString(), + }, + }); + expect(matchSessionByTarget(storage, { type: 'url', url: 'https://mcp.apify.com' }, {})).toBe( + '@apify' + ); + }); + + it('does not match when profile differs', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + profileName: 'work', + createdAt: new Date().toISOString(), + }, + }); + expect( + matchSessionByTarget(storage, { type: 'url', url: 'https://mcp.apify.com' }, {}) + ).toBeUndefined(); + expect( + matchSessionByTarget( + storage, + { type: 'url', url: 'https://mcp.apify.com' }, + { profile: 'work' } + ) + ).toBe('@apify'); + }); + + it('matches header key sets', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { + url: 'https://mcp.apify.com', + headers: { 'X-Test': '' }, + }, + createdAt: new Date().toISOString(), + }, + }); + expect( + matchSessionByTarget( + storage, + { type: 'url', url: 'https://mcp.apify.com' }, + { headers: ['X-Test: foo'] } + ) + ).toBe('@apify'); + expect( + matchSessionByTarget( + storage, + { type: 'url', url: 'https://mcp.apify.com' }, + { headers: ['Y-Other: foo'] } + ) + ).toBeUndefined(); + }); +}); + +describe('pickAvailableSessionName', () => { + describe('alwaysSuffix=false (URL/config behaviour)', () => { + it('returns bare candidate when not taken', () => { + expect(pickAvailableSessionName(makeStorage({}), '@apify', false)).toBe('@apify'); + }); + + it('returns -2 when bare candidate is taken', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + createdAt: new Date().toISOString(), + }, + }); + expect(pickAvailableSessionName(storage, '@apify', false)).toBe('@apify-2'); + }); + + it('returns -3 when bare and -2 are taken', () => { + const storage = makeStorage({ + '@apify': { + name: '@apify', + server: { url: 'https://mcp.apify.com' }, + createdAt: new Date().toISOString(), + }, + '@apify-2': { + name: '@apify-2', + server: { url: 'https://mcp.apify.com' }, + createdAt: new Date().toISOString(), + }, + }); + expect(pickAvailableSessionName(storage, '@apify', false)).toBe('@apify-3'); + }); + }); + + describe('alwaysSuffix=true (inline command behaviour)', () => { + it('returns -1 even when bare candidate is free', () => { + // Per design decision 3: every inline-command session always gets a numeric suffix. + expect(pickAvailableSessionName(makeStorage({}), '@npx', true)).toBe('@npx-1'); + }); + + it('returns -2 when -1 is taken', () => { + const storage = makeStorage({ + '@npx-1': { + name: '@npx-1', + server: { command: 'npx', args: ['-y', 'foo'] }, + createdAt: new Date().toISOString(), + }, + }); + expect(pickAvailableSessionName(storage, '@npx', true)).toBe('@npx-2'); + }); + + it('returns -3 when -1 and -2 are taken', () => { + const storage = makeStorage({ + '@npx-1': { + name: '@npx-1', + server: { command: 'npx', args: ['-y', 'a'] }, + createdAt: new Date().toISOString(), + }, + '@npx-2': { + name: '@npx-2', + server: { command: 'npx', args: ['-y', 'b'] }, + createdAt: new Date().toISOString(), + }, + }); + expect(pickAvailableSessionName(storage, '@npx', true)).toBe('@npx-3'); + }); + + it('does not return bare @npx when alwaysSuffix=true, even if @npx exists', () => { + const storage = makeStorage({ + '@npx': { + name: '@npx', + server: { command: 'npx', args: [] }, + createdAt: new Date().toISOString(), + }, + }); + // The bare @npx is taken too, so it picks -1. + expect(pickAvailableSessionName(storage, '@npx', true)).toBe('@npx-1'); + }); + + it('returns undefined when all 99 suffixes are taken', () => { + const sessions: Record = {}; + for (let i = 1; i <= 99; i++) { + const name = `@npx-${i}`; + sessions[name] = { + name, + server: { command: 'npx', args: [String(i)] }, + createdAt: new Date().toISOString(), + }; + } + expect(pickAvailableSessionName(makeStorage(sessions), '@npx', true)).toBeUndefined(); + }); + }); +}); diff --git a/test/unit/lib/utils.test.ts b/test/unit/lib/utils.test.ts index 5009f80f..6bbca1b1 100644 --- a/test/unit/lib/utils.test.ts +++ b/test/unit/lib/utils.test.ts @@ -16,6 +16,7 @@ import { getServerHost, isValidSessionName, generateSessionName, + shellSplit, isValidProfileName, validateProfileName, isValidResourceUri, @@ -396,6 +397,129 @@ describe('generateSessionName', () => { } }); }); + + describe('inline command targets', () => { + it('should return basename for plain commands', () => { + expect(generateSessionName({ type: 'command', command: 'npx', args: [] })).toBe('@npx'); + expect(generateSessionName({ type: 'command', command: 'node', args: ['dist/foo.js'] })).toBe( + '@node' + ); + }); + + it('should strip directory prefix from command path', () => { + expect( + generateSessionName({ type: 'command', command: '/usr/local/bin/python', args: [] }) + ).toBe('@python'); + expect(generateSessionName({ type: 'command', command: './my-mcp-server', args: [] })).toBe( + '@my-mcp-server' + ); + }); + + it('should strip Windows-style backslash directory prefix', () => { + expect( + generateSessionName({ type: 'command', command: 'C:\\tools\\node.exe', args: [] }) + ).toBe('@node'); + }); + + it('should strip common script extensions', () => { + expect(generateSessionName({ type: 'command', command: 'server.py', args: [] })).toBe( + '@server' + ); + expect(generateSessionName({ type: 'command', command: 'app.js', args: [] })).toBe('@app'); + expect(generateSessionName({ type: 'command', command: 'tool.mjs', args: [] })).toBe('@tool'); + expect(generateSessionName({ type: 'command', command: 'run.sh', args: [] })).toBe('@run'); + }); + + it('should sanitize special characters', () => { + expect(generateSessionName({ type: 'command', command: 'weird name!', args: [] })).toBe( + '@weird-name' + ); + }); + + it('should fall back to "session" for empty/invalid command basename', () => { + expect(generateSessionName({ type: 'command', command: '!!!', args: [] })).toBe('@session'); + }); + + it('should produce valid session names', () => { + const commands = ['npx', 'node', 'python', 'mcp-fs', './my-mcp-server', 'server.py']; + for (const command of commands) { + const name = generateSessionName({ type: 'command', command, args: [] }); + expect(isValidSessionName(name)).toBe(true); + } + }); + }); +}); + +describe('shellSplit', () => { + it('returns empty array for empty input', () => { + expect(shellSplit('')).toEqual([]); + expect(shellSplit(' ')).toEqual([]); + }); + + it('splits on whitespace', () => { + expect(shellSplit('a b c')).toEqual(['a', 'b', 'c']); + expect(shellSplit('npx -y foo')).toEqual(['npx', '-y', 'foo']); + }); + + it('collapses consecutive whitespace', () => { + expect(shellSplit('a b\t\tc')).toEqual(['a', 'b', 'c']); + }); + + it('handles double-quoted tokens with spaces', () => { + expect(shellSplit('node "my server.js"')).toEqual(['node', 'my server.js']); + expect(shellSplit('"a b" "c d"')).toEqual(['a b', 'c d']); + }); + + it('handles single-quoted tokens (literal, no escapes)', () => { + expect(shellSplit("python -c 'import x; x.run()'")).toEqual([ + 'python', + '-c', + 'import x; x.run()', + ]); + }); + + it('handles escaped quotes inside double quotes', () => { + expect(shellSplit('echo "say \\"hi\\""')).toEqual(['echo', 'say "hi"']); + }); + + it('handles backslash escapes for backslash, dollar, backtick inside double quotes', () => { + expect(shellSplit('echo "\\\\"')).toEqual(['echo', '\\']); + expect(shellSplit('echo "\\$VAR"')).toEqual(['echo', '$VAR']); + expect(shellSplit('echo "\\`cmd\\`"')).toEqual(['echo', '`cmd`']); + }); + + it('preserves ${VAR} literals (no expansion)', () => { + expect(shellSplit('node ${PWD}/foo')).toEqual(['node', '${PWD}/foo']); + expect(shellSplit("node '${PWD}/foo'")).toEqual(['node', '${PWD}/foo']); + expect(shellSplit('node "${PWD}/foo"')).toEqual(['node', '${PWD}/foo']); + }); + + it('handles backslash escapes outside quotes', () => { + expect(shellSplit('echo path\\ with\\ spaces')).toEqual(['echo', 'path with spaces']); + expect(shellSplit('echo \\"hi\\"')).toEqual(['echo', '"hi"']); + }); + + it('preserves @scope/pkg-style tokens', () => { + expect(shellSplit('npx -y "@scope/pkg" foo')).toEqual(['npx', '-y', '@scope/pkg', 'foo']); + expect(shellSplit('npx -y @scope/pkg foo')).toEqual(['npx', '-y', '@scope/pkg', 'foo']); + }); + + it('throws on unclosed double quote', () => { + expect(() => shellSplit('echo "unclosed')).toThrow(/Unbalanced double quote/); + }); + + it('throws on unclosed single quote', () => { + expect(() => shellSplit("echo 'unclosed")).toThrow(/Unbalanced single quote/); + }); + + it('throws on trailing backslash', () => { + expect(() => shellSplit('echo foo\\')).toThrow(/Trailing backslash/); + }); + + it('handles empty quoted token', () => { + expect(shellSplit('echo "" foo')).toEqual(['echo', '', 'foo']); + expect(shellSplit("echo '' foo")).toEqual(['echo', '', 'foo']); + }); }); describe('isValidProfileName', () => {