Skip to content

Conversation

@JoJoJoJoJoJoJo
Copy link
Contributor

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo commented Sep 13, 2025

User description

Resolves #228


PR Type

Bug fix


Description

  • Fix invalid working directory issue in CLI

  • Add error handling for OSError when getting current directory

  • Automatically change to home directory when current directory is invalid

  • Display warning message to user when directory change occurs


Diagram Walkthrough

flowchart LR
  A["CLI Start"] --> B["Check Current Directory"]
  B --> C{"Directory Valid?"}
  C -->|Yes| D["Continue Normal Flow"]
  C -->|No| E["Change to Home Directory"]
  E --> F["Show Warning Message"]
  F --> D
Loading

File Walkthrough

Relevant files
Bug fix
cli.py
Add working directory validation and fallback                       

src/mcpm/cli.py

  • Add imports for os and Path modules
  • Add try-catch block to handle invalid working directory
  • Change to home directory when current directory is invalid
  • Display warning message to user when directory change occurs
+17/-0   

@qodo-merge-pro
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

228 - Partially compliant

Compliant requirements:

  • Ensure the CLI works properly when launched from clients that may provide an invalid or missing working directory.
  • Provide clear user feedback/logging when fallback behavior is applied.

Non-compliant requirements:

  • Address or explain the "Warning: Input is not a terminal (fd=0)" error noted in the logs.

Requires further human verification:

  • Validate the fix on Windsurf, Raycast, and Jan AI to confirm the initialization/connectivity issue is resolved.
  • Confirm there are no regressions when running from a normal terminal and other environments.
  • Verify logs are clean (or at least non-fatal) and that the warning behavior is acceptable to users.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Import order

Standard library imports (os, pathlib) are placed after project imports; consider grouping stdlib imports at the top for clarity and consistency.

import os
from pathlib import Path
Robustness

Consider guarding os.chdir(home_dir) with a try/except and providing a secondary fallback (e.g., root or abort with clear error) if the home directory is inaccessible; optionally update PWD env var for consistency on POSIX shells.

try:
    # Check if the current working directory is valid.
    os.getcwd()
except OSError:
    # If getcwd() fails, it means the directory doesn't exist.
    # This can happen when mcpm is called from certain environments
    # like some Electron apps that don't set a valid cwd.
    home_dir = str(Path.home())
    err_console.print(
        f"Current working directory is invalid. Changing to home directory: {home_dir}",
        style="bold yellow"
    )
    os.chdir(home_dir)
Logging consistency

You print a warning via err_console.print; consider using the configured logging system for consistency and to respect verbosity/log level settings.

err_console.print(
    f"Current working directory is invalid. Changing to home directory: {home_dir}",
    style="bold yellow"
)

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Normalize CWD before imports

Move the current working directory (CWD) validation logic from inside the main
function to the top of the script. This should be done before any mcpm modules
are imported to prevent crashes during application initialization on systems
with an invalid CWD.

Examples:

src/mcpm/cli.py [12-104]
from mcpm.clients.client_config import ClientConfigManager
from mcpm.commands import (
    client,
    config,
    doctor,
    edit,
    info,
    inspect,
    install,
    list,

 ... (clipped 83 lines)

Solution Walkthrough:

Before:

# src/mcpm/cli.py

# mcpm modules are imported, potentially running code that uses CWD
from mcpm.clients.client_config import ClientConfigManager
from mcpm.commands import client, config, ...
# ... other mcpm imports

@click.group(...)
def main(ctx, version, help_flag):
    # CWD is checked here, but it might be too late
    try:
        os.getcwd()
    except OSError:
        home_dir = str(Path.home())
        os.chdir(home_dir)
    
    # ... rest of main function

After:

# src/mcpm/cli.py
import os
from pathlib import Path
import sys

# CWD normalization happens at the very top of the script
try:
    os.getcwd()
except OSError:
    home_dir = str(Path.home())
    os.chdir(home_dir)
    # A warning could be printed to sys.stderr here

# Now, import application modules safely
from mcpm.clients.client_config import ClientConfigManager
from mcpm.commands import client, config, ...
# ... other mcpm imports

@click.group(...)
def main(ctx, version, help_flag):
    # The CWD is already guaranteed to be valid here
    # ... rest of main function
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw where the CWD fix inside main() runs after module imports, leaving the application vulnerable to crashing at startup if any imported module relies on a valid CWD.

High
Possible issue
Handle chdir failures with fallback

If changing to the home directory fails, the CLI will still crash. Wrap
os.chdir(home_dir) in a try/except and add a fallback (e.g., filesystem root) to
ensure a safe working directory is set. Emit clear error messages when fallbacks
are used.

src/mcpm/cli.py [92-104]

 try:
     # Check if the current working directory is valid.
     os.getcwd()
 except OSError:
     # If getcwd() fails, it means the directory doesn't exist.
     # This can happen when mcpm is called from certain environments
     # like some Electron apps that don't set a valid cwd.
     home_dir = str(Path.home())
     err_console.print(
-        f"Current working directory is invalid. Changing to home directory: {home_dir}",
+        f"Current working directory is invalid. Attempting to change to home directory: {home_dir}",
         style="bold yellow"
     )
-    os.chdir(home_dir)
+    try:
+        if not os.path.isdir(home_dir):
+            raise OSError(f"Home directory does not exist: {home_dir}")
+        os.chdir(home_dir)
+    except OSError as e:
+        # Fall back to filesystem root as a last resort
+        fallback_dir = os.path.abspath(os.sep)
+        err_console.print(
+            f"Failed to change to home directory ({e}). Falling back to: {fallback_dir}",
+            style="bold red"
+        )
+        try:
+            os.chdir(fallback_dir)
+        except OSError:
+            err_console.print(
+                "Failed to change to a safe working directory; proceeding may fail.",
+                style="bold red"
+            )
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that os.chdir(home_dir) could fail, and proposes a robust fallback mechanism, enhancing the reliability of the CLI's startup sequence.

Medium
Guard home directory resolution

Path.home() can fail on some environments, causing a crash. Guard its resolution
with a try/except and fallback to os.path.expanduser("~") or filesystem root.

src/mcpm/cli.py [99-103]

-home_dir = str(Path.home())
+try:
+    home_dir = str(Path.home())
+except Exception:
+    # Fallback if home cannot be resolved
+    home_dir = os.path.expanduser("~") or os.path.abspath(os.sep)
 err_console.print(
     f"Current working directory is invalid. Changing to home directory: {home_dir}",
     style="bold yellow"
 )
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that Path.home() can fail in some environments and provides a sensible fallback, which improves the robustness of the error handling logic.

Medium
  • More

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo merged commit d54acd3 into main Sep 13, 2025
9 checks passed
@JoJoJoJoJoJoJo JoJoJoJoJoJoJo deleted the jonathan/fix-work-dir branch September 13, 2025 01:48
mcpm-semantic-release bot pushed a commit that referenced this pull request Sep 13, 2025
## [2.8.1](v2.8.0...v2.8.1) (2025-09-13)

### Bug Fixes

* **ci:** only trigger release for mcpm update ([#262](#262)) ([c1f02a3](c1f02a3))
* set working directory if current is invalid ([#261](#261)) ([d54acd3](d54acd3))
@mcpm-semantic-release
Copy link

🎉 This PR is included in version 2.8.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Issue on some MCP clients, including Windsurf

2 participants