Skip to content

fix: report missing startup dependencies instead of exiting with a bare traceback - #355

Merged
Jeomon merged 1 commit into
CursorTouch:mainfrom
dgonick:fix/startup-missing-dependency-message
Jul 30, 2026
Merged

fix: report missing startup dependencies instead of exiting with a bare traceback#355
Jeomon merged 1 commit into
CursorTouch:mainfrom
dgonick:fix/startup-missing-dependency-message

Conversation

@dgonick

@dgonick dgonick commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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() in try/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 at uv 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, sys and logger were already in scope; NoReturn keeps the fall-through after except statically sound.

Verification

Reproduced the real failure by renaming win32comext/ out of a synced venv, which yields the exact error from #345 at desktop/utils.py:7. Before the change the process exits on an unhandled traceback. After:

$ windows-mcp serve --transport stdio
exit code: 1
stdout:    (empty)
stderr:    windows-mcp: cannot start -- No module named 'win32com.shell'. The environment
           looks partially installed; run `uv sync` in the extension directory, then
           restart the client.

Full suite: 411 passed. ruff check: clean.

Known tradeoff in the catch

except ModuleNotFoundError around those three imports will also catch a ModuleNotFoundError raised inside windows_mcp.tools or windows_mcp.infrastructure — an internal import typo, for instance. In that case the message ("the environment looks partially installed; run uv sync") is wrong and points at a dead end.

This is deliberate. CI runs the suite on windows-latest and 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 inspecting exc.name against 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 shell from win32comext.shell instead of win32com.shell in desktop/utils.py. The __path__ splice in win32com/__init__.py exists so that win32com.shell is 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 runs uv sync --frozen on windows-latest with no post-install and imports desktop.utils via the test suite successfully. Packaging left alone.

🤖 Generated with Claude Code

_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>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix startup: actionable message for missing runtime dependencies

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Catch missing runtime dependencies during MCP server startup imports.
• Emit a single actionable stderr line (module + uv sync) and exit(1) instead of traceback.
• Add regression test ensuring non-zero exit, clean stdout, and helpful stderr.
Diagram

graph TD
  A["CLI entry (windows-mcp)"] --> B["__main__._build_mcp()"] --> C{Imports ok?}
  C -->|"Yes"| D["Create FastMCP server"] --> E["Register tools / lifespan"]
  C -->|"No (ModuleNotFoundError)"| F["__main__._exit_missing_dependency()"]
  F --> G["stderr message + exit(1)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Whitelist known third-party modules for the catch
  • ➕ Avoids mislabeling internal import bugs as “partial venv”
  • ➕ Keeps guidance accurate when the missing module is within project code
  • ➖ Requires maintaining an allowlist/mapping as dependencies evolve
  • ➖ More code/complexity than the current targeted fix
2. Validate environment proactively (dependency self-check)
  • ➕ Can detect and report multiple missing deps at once
  • ➕ Moves failure earlier and potentially clearer than import-time errors
  • ➖ Typically needs mapping import names to distributions (non-trivial)
  • ➖ Adds runtime overhead and more surface area for failures
3. Catch broader ImportError and re-raise with ClickException
  • ➕ Uniform UX for all import-time startup failures
  • ➕ Integrates with Click’s error formatting
  • ➖ Risks hiding useful tracebacks for unexpected import issues
  • ➖ Less specific than ModuleNotFoundError for the primary user failure mode

Recommendation: Current approach is appropriate for the stated scope: it fixes the user-facing “silent disconnect” by emitting one actionable stderr line while keeping stdout clean for stdio transport. If reviewers want to reduce the known tradeoff (internal import typo misreported as partial venv), the smallest follow-up would be a narrow allowlist check on exc.name, but it’s reasonable to defer until there’s evidence of confusion in practice.

Files changed (2) +45 / -4

Bug fix (1) +28 / -4
__main__.pyGuard startup imports and print actionable stderr on missing deps +28/-4

Guard startup imports and print actionable stderr on missing deps

• Adds '_exit_missing_dependency()' to convert 'ModuleNotFoundError' during startup imports into a single stderr message recommending 'uv sync', then exits with code 1. Wraps '_build_mcp()'’s critical imports in a try/except and logs the original exception at debug level.

src/windows_mcp/main.py

Tests (1) +17 / -0
test_startup_dependency_guard.pyTest missing-dependency guard exits(1) with clean stdout +17/-0

Test missing-dependency guard exits(1) with clean stdout

• Introduces a regression test that calls '_exit_missing_dependency()' with a representative 'ModuleNotFoundError'. Asserts exit code 1, stderr contains the missing module name and 'uv sync', and stdout remains empty to protect the MCP stdio stream.

tests/test_startup_dependency_guard.py

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 16 rules

Grey Divider


Remediation recommended

1. Overlong click.echo f-string 📘 Rule violation ✧ Quality
Description
A newly added f-string line exceeds the 100-character maximum, reducing readability and violating
the style limit.
Code

src/windows_mcp/main.py[196]

+        f"installed; run `uv sync` in the extension directory, then restart the client.",
Relevance

●●● Strong

Repo has accepted wrapping long lines to satisfy 100-char limit; this is a simple formatting change.

PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 222796 requires all changed, non-comment lines to be ≤100 characters. The added
click.echo message line is longer than 100 characters in src/windows_mcp/__main__.py.

Rule 222796: Enforce maximum line length of 100 characters
src/windows_mcp/main.py[196-196]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


2. Test missing type hints 📘 Rule violation ✧ Quality
Description
The newly added test function has no parameter or return type annotations, which violates the
requirement that all function signatures be type hinted.
Code

tests/test_startup_dependency_guard.py[6]

+def test_missing_dependency_exits_nonzero_with_actionable_stderr(capsys):
Relevance

●●● Strong

Precedent: reviewers accepted adding type annotations (incl. -> None) to test functions to satisfy
type-hint rule.

PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 222805 requires type hints for all function signatures. The added `def
test_missing_dependency_exits_nonzero_with_actionable_stderr(capsys):` lacks both a type annotation
for capsys and an explicit return annotation.

Rule 222805: Require type hints for all function signatures
tests/test_startup_dependency_guard.py[6-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

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.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

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):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

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

@Jeomon

Jeomon commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks

@Jeomon
Jeomon merged commit a1297da into CursorTouch:main Jul 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants