fix: report missing startup dependencies instead of exiting with a bare traceback - #355
Conversation
_build_mcp() imported the desktop service with no exception handling, so a partially populated venv produced an unhandled traceback and process exit. MCP hosts do not surface that traceback -- Claude Desktop shows only "Server disconnected" -- leaving no indication of which package is missing or how to recover. Catch ModuleNotFoundError around the startup imports and emit one actionable line on stderr naming the failing module and pointing at `uv sync`. stderr rather than stdout, because stdout carries the stdio protocol stream. Refs CursorTouch#345 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoFix startup: actionable message for missing runtime dependencies
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
16 rules 1. Overlong click.echo f-string
|
| logger.debug("Startup import failed", exc_info=exc) | ||
| click.echo( | ||
| f"windows-mcp: cannot start -- {exc}. The environment looks partially " | ||
| f"installed; run `uv sync` in the extension directory, then restart the client.", |
There was a problem hiding this comment.
1. Overlong click.echo f-string 📘 Rule violation ✧ Quality
A newly added f-string line exceeds the 100-character maximum, reducing readability and violating the style limit.
Agent Prompt
## Issue description
A newly added line in `_exit_missing_dependency()` exceeds the 100-character maximum line length.
## Issue Context
Compliance requires max 100 characters per non-comment line; the long `click.echo` message should be wrapped across lines without splitting tokens awkwardly.
## Fix Focus Areas
- src/windows_mcp/__main__.py[194-198]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| from windows_mcp import __main__ as wm | ||
|
|
||
|
|
||
| def test_missing_dependency_exits_nonzero_with_actionable_stderr(capsys): |
There was a problem hiding this comment.
2. Test missing type hints 📘 Rule violation ✧ Quality
The newly added test function has no parameter or return type annotations, which violates the requirement that all function signatures be type hinted.
Agent Prompt
## Issue description
The new test function is missing type hints for its parameter and return type.
## Issue Context
Compliance requires explicit type annotations on all `def` signatures (including tests).
## Fix Focus Areas
- tests/test_startup_dependency_guard.py[1-17]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Thanks |
Refs #345.
Problem
_build_mcp()imports the desktop service with no exception handling. On a partially populated venv a single missing dependency produces an unhandled traceback and process exit. MCP hosts do not surface that traceback — Claude Desktop shows only "Server disconnected" — so the user gets no indication of which package is missing or how to recover. The state does not self-heal: the venv looks built, so it is never rebuilt.Two independent triggers are known to produce the partially populated venv (see #345): an interpreter change invalidating the venv, and a cold build truncated by the client's initialize handshake timeout. This PR addresses neither trigger. It makes the resulting failure legible.
Change
Wrap the three startup imports in
_build_mcp()intry/except ModuleNotFoundError, and add_exit_missing_dependency(), which logs the exception at debug level, writes one line to stderr naming the failing module and pointing atuv sync, and exits 1. stderr rather than stdout, because stdout carries the stdio protocol stream.New test asserts exit code 1, that stderr names the failing module and the remedy, and that stdout stays empty.
No refactors, no new dependencies, no packaging changes.
click,sysandloggerwere already in scope;NoReturnkeeps the fall-through afterexceptstatically sound.Verification
Reproduced the real failure by renaming
win32comext/out of a synced venv, which yields the exact error from #345 atdesktop/utils.py:7. Before the change the process exits on an unhandled traceback. After:Full suite: 411 passed.
ruff check: clean.Known tradeoff in the catch
except ModuleNotFoundErroraround those three imports will also catch aModuleNotFoundErrorraised insidewindows_mcp.toolsorwindows_mcp.infrastructure— an internal import typo, for instance. In that case the message ("the environment looks partially installed; runuv sync") is wrong and points at a dead end.This is deliberate. CI runs the suite on
windows-latestand imports both packages, so an internal import error fails before release, whereas the partial-venv case only ever appears on user machines. Narrowing the catch by inspectingexc.nameagainst an allowlist of declared distributions is the scope creep this PR is trying to avoid. Happy to add that if you would rather have it.Considered and not included
Importing
shellfromwin32comext.shellinstead ofwin32com.shellindesktop/utils.py. The__path__splice inwin32com/__init__.pyexists so thatwin32com.shellis the supported public name, and when pywin32 is genuinely absent the change only alters which module name appears in the error. Left alone deliberately.The pywin32 post-install step (
pywin32_postinstall.py -install) is not run by uv or pip, but does not appear to be required here — CI runsuv sync --frozenonwindows-latestwith no post-install and importsdesktop.utilsvia the test suite successfully. Packaging left alone.🤖 Generated with Claude Code