From f3350efeb386b1faf33d093f97b4415f1f62bf50 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Mar 2026 02:22:19 +0000 Subject: [PATCH 1/2] fix: e2e test failures for schema-mode validation and run.sh portability - Align --schema-mode error message in getOptionsFromCommand() with test expectations (was "Invalid schema mode", now matches "Invalid --schema-mode value" format used elsewhere) - Add --timeout validation in getOptionsFromCommand() since validateArgValues() now stops at the first non-option arg and no longer catches subcommand options - Replace bash process substitution (< <(...)) in run.sh with heredoc alternatives (<<< "$()") for portability in environments without /dev/fd https://claude.ai/code/session_01FSt1NZy2bmcF2zDbhFEjSQ --- src/cli/index.ts | 12 ++++++++++-- test/e2e/run.sh | 30 ++++++++++-------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index 5a20595..acfc503 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -86,7 +86,15 @@ function getOptionsFromCommand(command: Command): HandlerOptions { // Always convert to array for consistent handling options.headers = Array.isArray(opts.header) ? opts.header : [opts.header]; } - if (opts.timeout) options.timeout = parseInt(opts.timeout, 10); + if (opts.timeout) { + const timeout = parseInt(opts.timeout as string, 10); + if (isNaN(timeout) || timeout <= 0) { + throw new Error( + `Invalid --timeout value: "${opts.timeout as string}". Must be a positive number (seconds).` + ); + } + options.timeout = timeout; + } if (opts.profile) options.profile = opts.profile; if (verbose) options.verbose = verbose; if (opts.x402) options.x402 = true; @@ -94,7 +102,7 @@ function getOptionsFromCommand(command: Command): HandlerOptions { if (opts.schemaMode) { const mode = opts.schemaMode as string; if (mode !== 'strict' && mode !== 'compatible' && mode !== 'ignore') { - throw new Error(`Invalid schema mode: ${mode}. Must be 'strict', 'compatible', or 'ignore'.`); + throw new Error(`Invalid --schema-mode value: "${mode}". Valid modes are: strict, compatible, ignore`); } options.schemaMode = mode; } diff --git a/test/e2e/run.sh b/test/e2e/run.sh index f1fc1f1..62cbce1 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run.sh @@ -118,37 +118,27 @@ done # Suites directory SUITES_DIR="$SCRIPT_DIR/suites" -# Find all test files matching patterns +# Find all test files matching patterns (outputs newline-separated paths) find_tests() { - local tests=() - if [[ ${#PATTERNS[@]} -eq 0 ]]; then # No pattern - find all tests in suites/ - while IFS= read -r -d '' test; do - tests+=("$test") - done < <(find "$SUITES_DIR" -name "*.test.sh" -print0 | sort -z) + find "$SUITES_DIR" -name "*.test.sh" | sort else for pattern in "${PATTERNS[@]}"; do if [[ -f "$SUITES_DIR/$pattern" ]]; then # Specific file - tests+=("$SUITES_DIR/$pattern") + echo "$SUITES_DIR/$pattern" elif [[ -d "$SUITES_DIR/$pattern" ]]; then # Directory - find all tests in it - while IFS= read -r -d '' test; do - tests+=("$test") - done < <(find "$SUITES_DIR/$pattern" -name "*.test.sh" -print0 | sort -z) + find "$SUITES_DIR/$pattern" -name "*.test.sh" | sort elif [[ -d "$SUITES_DIR/${pattern%/}" ]]; then # Directory without trailing slash - while IFS= read -r -d '' test; do - tests+=("$test") - done < <(find "$SUITES_DIR/${pattern%/}" -name "*.test.sh" -print0 | sort -z) + find "$SUITES_DIR/${pattern%/}" -name "*.test.sh" | sort else echo "Warning: No tests match pattern: $pattern" >&2 fi done fi - - printf '%s\n' "${tests[@]}" } # Get test name from path (relative to suites dir, without .test.sh) @@ -158,11 +148,11 @@ test_name() { echo "${rel%.test.sh}" } -# Collect tests (compatible with bash 3.x on macOS) +# Collect tests TESTS=() while IFS= read -r test; do [[ -n "$test" ]] && TESTS+=("$test") -done < <(find_tests) +done <<< "$(find_tests)" if [[ ${#TESTS[@]} -eq 0 ]]; then echo "No tests found" >&2 @@ -362,9 +352,9 @@ fi # Check for setup requirements (tests that were skipped due to missing configuration) SETUP_FILES=() -while IFS= read -r -d '' setup_file; do - SETUP_FILES+=("$setup_file") -done < <(find "$RUN_DIR" -name ".setup_required" -print0 2>/dev/null) +while IFS= read -r setup_file; do + [[ -n "$setup_file" ]] && SETUP_FILES+=("$setup_file") +done <<< "$(find "$RUN_DIR" -name ".setup_required" 2>/dev/null)" if [[ ${#SETUP_FILES[@]} -gt 0 ]]; then echo "" From c740b602b8b6ad8163ac7d94d0ad96c86943c47c Mon Sep 17 00:00:00 2001 From: Jan Curn Date: Sat, 7 Mar 2026 09:55:20 +0100 Subject: [PATCH 2/2] TODOs --- docs/TODOs.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/TODOs.md b/docs/TODOs.md index 69fd530..997235f 100644 --- a/docs/TODOs.md +++ b/docs/TODOs.md @@ -31,6 +31,13 @@ Run "mcpc --help" for usage information. +- mcpc @session --timeout ... / mcpc @session --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 + + ## x402 - sign -r Sign payment from PAYMENT-REQUIRED header - why the "-r" is needed? @@ -109,7 +116,7 @@ $ mcpc connect - "login" and "logout" commands could work also with file:entry, just use the remote server URL - maybe introduce new session status: auth failed or unauthed -- ux: consider forking "alive" session state to "alive" and "diconnected", to indicate the remove server is not responding but bridge + ux: consider forking "alive" session state to "alive" and "disconnected", to indicate the remote server is not responding but bridge runs fine. We can use lastSeenAt + ping interval info for that, or status of last ping. - ux: Be even more forgiving with `args:=x`, when we know from tools/prompt schema the text is compatible with `x` even if the exact type is not - just re-type it dynamically to make it work.