Merge branch 'main' into claude/fix-session-options-xZywy

Author: jancurn

CHANGELOG.md

@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `mcpc close @session`, `mcpc restart @session`, and `mcpc shell @session` command-first syntax as alternatives to `mcpc @session close/restart/shell`
 - E2E tests now run under the Bun runtime (in addition to Node.js); use `./test/e2e/run.sh --runtime bun` or `npm run test:e2e:bun`
 
+### Fixed
+- `parseServerArg()` now handles well Windows drive-letter config paths as well as other ambiguous cases
+
 ### Changed
 - **Breaking:** CLI syntax redesigned to command-first style. All commands now start with a verb; MCP operations require a named session.
 

docs/TODOs.md

@@ -33,10 +33,9 @@ Run "mcpc --help" for usage information.
 
 - mcpc @session --timeout ... / mcpc @session <cmd> --timeout ... has no effect
 
-- createSessionProgram() advertises --header and --profile options for mcpc @session ..., but these values are never applied: withMcpClient()/SessionClient ignore headers/profile overrides and always use the session’s stored config. This is misleading for users and makes it easy to think a command is authenticated/modified when it isn’t. Either wire these options into session execution (e.g. by updating/restarting the session/bridge) or remove them from the session program/help.
-
-- parseServerArg() splits config entries using the first : (arg.indexOf(':')). This breaks Windows paths with drive letters (e.g. C:\Users\me\mcp.json:filesystem), which would be parsed as file=C entry=\Users\.... Consider special-casing ^[A-Za-z]:[\\/] and/or using lastIndexOf(':') for the file/entry delimiter to keep Windows paths working
-- parseServerArg() treats any string containing : (that wasn’t recognized as a URL) as a config file:entry. This will misclassify inputs like example.com:foo (invalid host:port) as a config file named example.com. Consider tightening the config heuristic (e.g. require the left side to look like a file path or have a known config extension) and return null for ambiguous/invalid host:port inputs.
+IN PROGRESS - createSessionProgram() advertises --header and --profile options for mcpc @session ..., but these values are never applied: 
+withMcpClient()
+/SessionClient ignore headers/profile overrides and always use the session’s stored config. This is misleading for users and makes it easy to think a command is authenticated/modified when it isn’t. Either wire these options into session execution (e.g. by updating/restarting the session/bridge) or remove them from the session program/help.
 
 - validateOptions() relies on KNOWN_OPTIONS, but several options used by subcommands are missing (e.g. --scope on login, -r/--payment-required, --amount, --expiry for x402 sign, and session flags like -o/--output, --max-size). This will cause valid commands to fail early with "Unknown option" before routing to the correct Commander program. Either expand KNOWN_OPTIONS to cover all CLI flags (including subcommand-specific ones) or change validation to only check global options (e.g. only scan args before the first non-option command token
 - login introduces a --scope option here, but the pre-parse validateOptions() step uses KNOWN_OPTIONS from parser.ts, which currently does not include --scope. As a result, mcpc login <server> --scope ... will fail early with "Unknown option: --scope" before Commander runs. Add --scope to the known options list or make option validation command-aware.

package.json

@@ -32,7 +32,7 @@
     "build": "tsc",
     "build:watch": "tsc --watch",
     "build:readme": "./scripts/update-readme.sh",
-    "test": "npm run build && npm run test:unit && ./test/e2e/run.sh --no-build --parallel 4 && ./test/e2e/run.sh --no-build --runtime bun",
+    "test": "npm run build && npm run test:unit && ./test/e2e/run.sh --no-build --parallel 4 && ./test/e2e/run.sh --no-build --parallel 4 --runtime bun",
     "test:unit": "jest",
     "test:watch": "jest --watch",
     "test:coverage": "npm run test:coverage:unit && npm run test:coverage:e2e && npm run test:coverage:merge",

src/cli/parser.ts

@@ -277,14 +277,34 @@ function isValidUrlWithHost(str: string): boolean {
   }
 }
 
+/**
+ * Returns true if s looks like a filesystem path rather than a hostname.
+ * Used to decide whether the left side of a colon is a file path or a host.
+ */
+function looksLikeFilePath(s: string): boolean {
+  // Unix absolute or home-relative paths
+  if (s.startsWith('/') || s.startsWith('~')) return true;
+  // Explicit relative paths
+  if (s.startsWith('./') || s.startsWith('../')) return true;
+  // Windows absolute paths: C:\ or C:/
+  if (/^[A-Za-z]:[/\\]/.test(s)) return true;
+  // Contains a directory separator (e.g. subdir/file.json)
+  if (s.includes('/') || s.includes('\\')) return true;
+  // Known config file extensions without any path prefix
+  if (/\.(json|yaml|yml)$/i.test(s)) return true;
+  return false;
+}
+
 /**
  * Parse a server argument into a URL or config file entry.
  *
  * 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
  *    (e.g. https://~/ or https:///// parse with unusual hosts but are clearly file paths).
- * 2. Config entry: colon present with non-empty text on both sides  →  file:entry
- * 3. Otherwise: returns null (caller should report an error)
+ * 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)
  */
 export function parseServerArg(
   arg: string
@@ -294,27 +314,39 @@ export function parseServerArg(
     return { type: 'url', url: arg };
   }
 
-  // Step 1b: try adding https:// / http:// prefix for bare hostnames and host:port combos.
+  // Step 1b: if arg contains "://" it's clearly intended as a full URL — don't fall through to
+  // the config-entry heuristic (e.g. "https://host:badport" should not become file="https").
+  if (arg.includes('://')) {
+    return null;
+  }
+
+  // Step 2: try adding https:// prefix for bare hostnames and host:port combos.
   // Skip if arg starts with a path character — those are file paths, not hostnames.
   // Skip if arg ends with ':' — dangling colon is not a valid hostname.
-  const startsWithPathChar = arg.startsWith('/') || arg.startsWith('~') || arg.startsWith('.');
+  const isWindowsDrive = /^[A-Za-z]:[/\\]/.test(arg);
+  const startsWithPathChar =
+    arg.startsWith('/') || arg.startsWith('~') || arg.startsWith('.') || isWindowsDrive;
   if (!startsWithPathChar && !arg.endsWith(':')) {
-    if (isValidUrlWithHost('https://' + arg) || isValidUrlWithHost('http://' + arg)) {
+    if (isValidUrlWithHost('https://' + arg)) {
       return { type: 'url', url: arg };
     }
   }
 
-  // Step 2: config file entry — colon with non-empty text on both sides
-  const colonIndex = arg.indexOf(':');
+  // Step 3: config file entry — colon separates file path from entry name.
+  // The left side must look like a file path (not a bare hostname).
+  // Special case: Windows drive-letter paths (C:\...) have a colon at position 1;
+  // use lastIndexOf(':') so we skip that drive colon and find the entry separator.
+  const colonIndex = isWindowsDrive ? arg.lastIndexOf(':') : arg.indexOf(':');
+
   if (colonIndex > 0 && colonIndex < arg.length - 1) {
-    return {
-      type: 'config',
-      file: arg.substring(0, colonIndex),
-      entry: arg.substring(colonIndex + 1),
-    };
+    const file = arg.substring(0, colonIndex);
+    const entry = arg.substring(colonIndex + 1);
+    if (looksLikeFilePath(file)) {
+      return { type: 'config', file, entry };
+    }
   }
 
-  // Step 3: unrecognised
+  // Step 4: unrecognised
   return null;
 }
 

test/unit/cli/index.test.ts

@@ -51,6 +51,9 @@ describe('parseServerArg', () => {
 
     const result2 = parseServerArg('http://mcp.apify.com');
     expect(result2).toEqual({ type: 'url', url: 'http://mcp.apify.com' });
+
+    const result3 = parseServerArg('http://mcp.apify.com:8000');
+    expect(result3).toEqual({ type: 'url', url: 'http://mcp.apify.com:8000' });
   });
 
   it('should parse a URL with path (no colon-entry) as URL', () => {
@@ -59,6 +62,9 @@ describe('parseServerArg', () => {
 
     const result2 = parseServerArg('mcp.apify.com/v1');
     expect(result2).toEqual({ type: 'url', url: 'mcp.apify.com/v1' });
+
+    const result3 = parseServerArg('mcp.apify.com:8000/v1');
+    expect(result3).toEqual({ type: 'url', url: 'mcp.apify.com:8000/v1' });
   });
 
   it('should parse ~/.vscode/mcp.json:filesystem as config', () => {
@@ -76,14 +82,31 @@ describe('parseServerArg', () => {
     expect(result).toEqual({ type: 'config', file: '/absolute/path.json', entry: 'entry' });
   });
 
+  it('should parse .json extension as config', () => {
+    const result = parseServerArg('./config.json:myserver');
+    expect(result).toEqual({ type: 'config', file: './config.json', entry: 'myserver' });
+
+    const result2 = parseServerArg('config.json:myserver');
+    expect(result2).toEqual({ type: 'config', file: 'config.json', entry: 'myserver' });
+  });
+
   it('should parse .yaml extension as config', () => {
     const result = parseServerArg('./config.yaml:myserver');
     expect(result).toEqual({ type: 'config', file: './config.yaml', entry: 'myserver' });
+
+    const result2 = parseServerArg('config.yaml:myserver');
+    expect(result2).toEqual({ type: 'config', file: 'config.yaml', entry: 'myserver' });
   });
 
   it('should parse .yml extension as config', () => {
-    const result = parseServerArg('config.yml:myserver');
-    expect(result).toEqual({ type: 'config', file: 'config.yml', entry: 'myserver' });
+    const result = parseServerArg('./config.yml:myserver');
+    expect(result).toEqual({ type: 'config', file: './config.yml', entry: 'myserver' });
+
+    const result2 = parseServerArg('config.yml:myserver');
+    expect(result2).toEqual({ type: 'config', file: 'config.yml', entry: 'myserver' });
+
+    const result3 = parseServerArg('../config.yml:myserver');
+    expect(result3).toEqual({ type: 'config', file: '../config.yml', entry: 'myserver' });
   });
 
   it('should NOT parse hostname:port as config', () => {
@@ -108,6 +131,32 @@ describe('parseServerArg', () => {
   it('should return null for trailing-colon input', () => {
     expect(parseServerArg('file:')).toBeNull();
   });
+
+  it('should return null for hostname:non-numeric-port (not a valid URL or file path)', () => {
+    expect(parseServerArg('example.com:foo')).toBeNull();
+    expect(parseServerArg('myhost:notaport')).toBeNull();
+  });
+
+  it('should return null for https:// URL with invalid port', () => {
+    expect(parseServerArg('https://mcp.apify.com:invalid')).toBeNull();
+    expect(parseServerArg('http://example.com:badport')).toBeNull();
+  });
+
+  it('should return null for other invalid full-URL syntax', () => {
+    expect(parseServerArg('https://host:badport/path')).toBeNull();
+  });
+
+  it('should parse Windows drive-letter config paths correctly', () => {
+    const result = parseServerArg('C:\\Users\\me\\mcp.json:filesystem');
+    expect(result).toEqual({
+      type: 'config',
+      file: 'C:\\Users\\me\\mcp.json',
+      entry: 'filesystem',
+    });
+
+    const result2 = parseServerArg('D:/projects/config.yaml:myserver');
+    expect(result2).toEqual({ type: 'config', file: 'D:/projects/config.yaml', entry: 'myserver' });
+  });
 });
 
 describe('extractOptions', () => {