Skip to content

feat: add --config-path flag to claude-desktop install command#3380

Merged
jlowin merged 6 commits intoPrefectHQ:mainfrom
Sumanshu-Nankana:feature/claude-desktop-config-path
Mar 4, 2026
Merged

feat: add --config-path flag to claude-desktop install command#3380
jlowin merged 6 commits intoPrefectHQ:mainfrom
Sumanshu-Nankana:feature/claude-desktop-config-path

Conversation

@Sumanshu-Nankana
Copy link
Copy Markdown
Contributor

Description

Currently, fastmcp install claude-desktop fails for users who have Claude Desktop installed in a non-standard location, because the tool only checks hardcoded OS-specific paths.

This PR adds an optional --config-path flag to the fastmcp install claude-desktop command, allowing users to manually specify their Claude Desktop config directory.

Changes made:

  • Added --config-path parameter to claude_desktop_command()
  • Updated get_claude_config_path() to accept an optional Path override
  • Updated install_claude_desktop() to pass config_path through
  • Added tests for --config-path parsing and default None behaviour

Contributors Checklist

Review Checklist

  • I have self-reviewed my changes
  • My Pull Request is ready for review

Screenshots

Testing --config-path flag locally (shows both default failure and successful install with custom path):

Screenshot 2026-03-03 222025 Screenshot 2026-03-03 222044

@marvin-context-protocol marvin-context-protocol Bot added cli Related to FastMCP CLI commands (run, dev, install) or CLI functionality. enhancement Improvement to existing functionality. For issues and smaller PR improvements. labels Mar 3, 2026
@Sumanshu-Nankana Sumanshu-Nankana changed the title Feature/claude desktop config path feat: add --config-path flag to claude-desktop install command Mar 3, 2026
Copy link
Copy Markdown
Member

@jlowin jlowin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The plumbing looks good.

One thing I'd like to see fixed before merging: when a user provides --config-path and the path doesn't exist, they currently get the generic "Claude Desktop config directory not found, please ensure Claude Desktop is installed..." message — which is confusing since they explicitly told us where to look. Could you add a distinct error for that case? Something like "The specified config path does not exist: {path}".

@Sumanshu-Nankana
Copy link
Copy Markdown
Contributor Author

Implemented in d7aebb3

Screenshot 2026-03-04 111931

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: The Windows CI job failed due to a test timeout in tests/server/tasks/test_task_meta_parameter.py, unrelated to the PR's changes (which add a --config-path CLI flag).

Root Cause: The 6th test in test_task_meta_parameter.py (the ttl_tool-related test) hung indefinitely on Windows after 5 tests passed. The thread dump shows all multiprocessing pool workers blocked on _winapi.WaitForMultipleObjects, a Windows-specific IPC primitive. The debug logs show a call_tool ttl_tool operation that was cancelled mid-flight, after which the notification subscriber infrastructure entered a deadlock state. This is a Windows/asyncio/multiprocessing interaction issue in the task notification subscriber machinery — not caused by this PR.

Suggested Solution: This appears to be a pre-existing flaky test on Windows, unrelated to the --config-path changes in this PR. The maintainers should:

  1. Investigate src/fastmcp/server/tasks/notifications.py for Windows-specific deadlock behavior in the notification subscriber
  2. Consider adding a pytest.mark.skipif(sys.platform == 'win32', ...) to affected tests in test_task_meta_parameter.py if the functionality isn't expected to work on Windows, OR
  3. Re-run the CI to see if this is a transient flake

This PR's changes (CLI flag addition) are not the cause.

Detailed Analysis

Failed job: Tests: Python 3.10 on windows-latest (job ID 65704047988)

Timeout log excerpt:

[03/04/26 11:46:58] DEBUG    Started notification subscriber for session ac02dadb-d3c5-4a8e-ac4a-6d625fed5dd5
[03/04/26 11:46:59] DEBUG    Starting notification subscriber for session ac02dadb-d3c5-4a8e-ac4a-6d625fed5dd5
tests\server\tasks\test_task_meta_parameter.py .....[TIMEOUT]

Thread dump shows all workers blocked:

Stack of Thread-2082 (_handle_workers):
  multiprocessing/pool.py:502 → wait(sentinels, timeout=timeout)
  multiprocessing/connection.py:879 → _exhaustive_wait(...)
  multiprocessing/connection.py:811 → _winapi.WaitForMultipleObjects(L, False, timeout)

Test configuration:

  • Timeout: 5 seconds (--timeout=5)
  • Platform: win32, Python 3.10.11
  • Parallel workers: up to 4 (--numprocesses auto --maxprocesses 4 --dist worksteal)
  • All Linux jobs passed; only Windows failed
Related Files
  • tests/server/tasks/test_task_meta_parameter.py — the test file with the hanging test
  • src/fastmcp/server/tasks/notifications.py — contains notification subscriber logic shown in the deadlock
  • src/fastmcp/server/tasks/handlers.py — task call handling
  • src/fastmcp/server/tasks/elicitation.py — also uses notification subscriber pattern

Copy link
Copy Markdown
Member

@jlowin jlowin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jlowin jlowin merged commit 34d848b into PrefectHQ:main Mar 4, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Related to FastMCP CLI commands (run, dev, install) or CLI functionality. enhancement Improvement to existing functionality. For issues and smaller PR improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow custom config path for 'fastmcp install claude-desktop'

2 participants