Fix corrupted comtypes.gen cache from concurrent generation

[pranco388]

Fixes #357

Problem

When the MCP host (e.g. Claude Desktop) spawns multiple server instances near-simultaneously, both processes generate the comtypes cache in comtypes/gen concurrently on first run. Concurrent writes leave the GUID module and the friendly wrapper (UIAutomationClient.py) mismatched or partial. Every subsequent startup then crashes with ImportError/AttributeError until the cache is manually deleted. Recurs after every venv rebuild.

Fix

New module windows_mcp/uia/comtypes_cache.py providing safe_get_module():

1. Cross-process file lock (msvcrt, lockfile in temp keyed by sys.prefix) so only one process generates the cache at a time.
2. Corrupt-cache recovery: on ImportError/AttributeError (or when the generated module lacks an expected attribute) the generated files are cleared, sys.modules entries purged, and generation retried once.

Used at both generation sites: uia/core.py (UIAutomationCore.dll) and tree/ia2.py (oleacc.dll). This also self-heals installs that already have a corrupted cache — no manual venv deletion needed.

Testing

• Clean generation: OK
• Corrupted cache (truncated GUID module) -> automatic recovery: OK
• 4 concurrent processes generating on an empty cache (the original race): all OK

[qodo-code-review]

PR Summary by Qodo

Prevent comtypes.gen corruption with cross-process locking and self-healing regeneration

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Serialize comtypes wrapper generation to avoid concurrent cache writes on first run.
• Detect and recover from corrupted comtypes.gen modules by clearing and regenerating once.
• Apply the safe wrapper generation to both UIAutomationCore.dll and oleacc.dll loaders.

Diagram

graph TD
  A["uia/core.py"] --> B["uia/comtypes_cache.py"] --> C["comtypes.client.GetModule"] --> D[("comtypes/gen cache")]
  E["tree/ia2.py"] --> B
  B --> F[("temp lock file")]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a dedicated cross-platform lock library (filelock/portalocker)

• ➕ Avoids bespoke locking loop and potential busy-spin behavior
• ➕ Can provide clearer semantics (timeouts, stale lock handling) and better portability
• ➖ Adds a new runtime dependency for a Windows-specific issue
• ➖ Still needs careful per-venv scoping and error recovery logic

2. Lock per type library rather than per venv-wide generation

• ➕ Allows concurrent generation of unrelated type libraries in different processes
• ➕ Reduces contention if many COM libraries are used at startup
• ➖ More complex lock naming and recovery logic
• ➖ Race/corruption often involves shared comtypes.gen state; per-lib locking may not be sufficient

Recommendation: The PR’s approach (single per-venv cross-process lock + one-shot cache purge/regenerate on corruption signals) is appropriate for a startup-critical reliability bug. It keeps behavior deterministic under concurrent launches and provides self-healing for already-corrupted environments without adding dependencies. If lock contention becomes an issue, consider adding a small sleep/backoff in the lock retry loop, but the overall strategy is sound.

Files changed (3) +104 / -2

Bug fix (3) +104 / -2

ia2.pyUse safe_get_module for oleacc.dll wrapper generation +3/-1

Use safe_get_module for oleacc.dll wrapper generation

• Replaces direct comtypes.client.GetModule("oleacc.dll") with safe_get_module(). Adds a required attribute check (IAccessible) to detect partially generated/corrupt modules before importing from comtypes.gen.

src/windows_mcp/tree/ia2.py

comtypes_cache.pyAdd cross-process locking + corrupt-cache recovery for comtypes.gen +97/-0

Add cross-process locking + corrupt-cache recovery for comtypes.gen

• Introduces safe_get_module() which wraps comtypes.client.GetModule with an msvcrt file lock (scoped to the current venv via sys.prefix hashing). On ImportError/AttributeError or missing expected attributes, it clears generated comtypes.gen files, purges related sys.modules entries, and retries once.

src/windows_mcp/uia/comtypes_cache.py

core.pySerialize UIAutomationCore.dll module generation via safe_get_module +4/-1

Serialize UIAutomationCore.dll module generation via safe_get_module

• Imports safe_get_module and uses it to load UIAutomationCore.dll instead of calling comtypes.client.GetModule directly. Adds a required attribute check (IUIAutomation) so corrupted generated modules are detected and regenerated.

src/windows_mcp/uia/core.py

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 16 rules

1. Non-Google docstrings in functions ✓ Resolved 📘 Rule violation ✧ Quality

Description

The new public functions safe_get_module() and comtypes_cache_lock() do not provide Google-style
docstrings (missing structured sections like Args:, Returns:, and Raises:). This violates the
requirement for Google-style docstrings on public functions in changed files.

Code

src/windows_mcp/uia/comtypes_cache.py[R77-83]

+def safe_get_module(tlib, required_attr=None):
+    """``comtypes.client.GetModule`` with locking and corrupt-cache recovery.
+
+    ``required_attr`` optionally names an attribute the generated module must
+    expose; a partially generated cache that imports but lacks it is treated
+    as corrupted and regenerated as well.
+    """

Relevance

●●● Strong

Google-style docstrings for public functions are enforced and previously accepted when requested.

PR-#353

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 222802 requires Google-style docstrings on public functions/classes in changed
files. The added functions have docstrings, but they are not in Google style and omit required
structured sections (e.g., Args:, Returns:, Raises:).

Rule 222802: Require Google-style docstrings on public functions and classes
src/windows_mcp/uia/comtypes_cache.py[31-33]
src/windows_mcp/uia/comtypes_cache.py[77-83]

Agent prompt

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

## Issue description
New public functions are missing Google-style docstrings with required sections.

## Issue Context
`safe_get_module()` is imported and used from other modules in this PR, making it part of the module’s public surface.

## Fix Focus Areas
- src/windows_mcp/uia/comtypes_cache.py[31-33]
- src/windows_mcp/uia/comtypes_cache.py[77-83]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. safe_get_module lacks type hints ✓ Resolved 📘 Rule violation ✧ Quality

Description

safe_get_module() and comtypes_cache_lock() are newly added without parameter and return type
annotations. This violates the requirement that all function signatures in changed code include
explicit type hints, reducing static analyzability and readability.

Code

src/windows_mcp/uia/comtypes_cache.py[R77-83]

+def safe_get_module(tlib, required_attr=None):
+    """``comtypes.client.GetModule`` with locking and corrupt-cache recovery.
+
+    ``required_attr`` optionally names an attribute the generated module must
+    expose; a partially generated cache that imports but lacks it is treated
+    as corrupted and regenerated as well.
+    """

Relevance

●●● Strong

Team enforces type hints on changed signatures; missing annotations usually get added.

PR-#353
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 functions
comtypes_cache_lock() and safe_get_module(tlib, required_attr=None) have no parameter
annotations and no return type annotation.

Rule 222805: Require type hints for all function signatures
src/windows_mcp/uia/comtypes_cache.py[31-33]
src/windows_mcp/uia/comtypes_cache.py[77-83]

Agent prompt

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

## Issue description
New functions `comtypes_cache_lock()` and `safe_get_module()` are missing type annotations for parameters and return types.

## Issue Context
The project requires explicit type hints on all function signatures added/modified in the PR.

## Fix Focus Areas
- src/windows_mcp/uia/comtypes_cache.py[31-50]
- src/windows_mcp/uia/comtypes_cache.py[77-97]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unguarded gen_dir listdir ✓ Resolved 🐞 Bug ☼ Reliability

Description

_clear_gen_cache() calls os.listdir(gen_dir) without handling OSError, so the self-healing path can
crash and skip regeneration when the cache directory is missing/unreadable. This can replace the
original ImportError/AttributeError with an uncaught exception and leave the cache corrupted.

Code

src/windows_mcp/uia/comtypes_cache.py[61]

+    for name in os.listdir(gen_dir):

Relevance

●●● Strong

Likely accepted: small defensive OSError handling to keep self-healing path from crashing.

PR-#232
PR-#238

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The recovery path in safe_get_module() calls _clear_gen_cache() after
ImportError/AttributeError, but _clear_gen_cache() can itself raise on os.listdir(gen_dir),
preventing the retry attempt from running.

src/windows_mcp/uia/comtypes_cache.py[53-75]
src/windows_mcp/uia/comtypes_cache.py[84-97]

Agent prompt

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

### Issue description
`_clear_gen_cache()` suppresses errors for individual deletions, but `os.listdir(gen_dir)` is unguarded. If listing fails (missing directory, permissions, transient FS issues), `_clear_gen_cache()` raises and the intended “clear + retry once” behavior in `safe_get_module()` is bypassed.

### Issue Context
This code runs specifically on the corruption-recovery path, so it should be best-effort and not introduce new fatal exceptions.

### Fix Focus Areas
- src/windows_mcp/uia/comtypes_cache.py[53-75]
- src/windows_mcp/uia/comtypes_cache.py[84-97]

Suggested implementation direction:
- Wrap `os.listdir(gen_dir)` in `try/except OSError` (or broader) and return early (optionally with a debug log) so `safe_get_module()` can proceed to its retry.
- Consider also guarding `os.path.dirname(gen.__file__)` if `__file__` is unexpectedly missing/None.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (1)

4. Unbounded lock retry ✓ Resolved 🐞 Bug ☼ Reliability

Description

comtypes_cache_lock() retries indefinitely on any OSError from msvcrt.locking(), so a permanent
locking failure (not just contention) can stall safe_get_module() (and thus UIA initialization)
forever. Treating all OS errors as transient also hides the real underlying failure mode from
callers.

Code

src/windows_mcp/uia/comtypes_cache.py[R34-43]

+    fd = os.open(_LOCK_PATH, os.O_CREAT | os.O_RDWR)
+    try:
+        while True:
+            try:
+                # Blocks and retries for ~10s, then raises OSError; loop
+                # until the other process releases the lock.
+                msvcrt.locking(fd, msvcrt.LK_LOCK, 1)
+                break
+            except OSError:
+                continue

Relevance

●● Moderate

Reliability concern but timeout/error-code policy is behavioral; no close precedent on lock retry
semantics.

PR-#238
PR-#232

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The lock acquisition loop catches all OSError and continues forever, with no timeout or error-code
filtering; because safe_get_module() is called during UIA client initialization, a stuck lock
acquisition blocks initialization of the automation client.

src/windows_mcp/uia/comtypes_cache.py[31-50]
src/windows_mcp/uia/core.py[49-76]

Agent prompt

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

### Issue description
`comtypes_cache_lock()` loops forever on **any** `OSError` raised by `msvcrt.locking()`. Some `OSError`s are not lock-contention (e.g., permissions/invalid handle/path issues), and in those cases the code will never make progress and will block UIA initialization indefinitely.

### Issue Context
The lock is used by `safe_get_module()`, which is called during `_AutomationClient` construction; a hang here blocks UIA usage entirely.

### Fix Focus Areas
- src/windows_mcp/uia/comtypes_cache.py[31-50]

Suggested implementation direction:
- Add a bounded timeout (e.g., `time.monotonic()` + max wait) and raise a clear exception when exceeded.
- Distinguish retryable contention errors from non-retryable errors (e.g., check `exc.winerror`/`exc.errno` when available) and re-raise immediately for non-contention failures.
- Optionally add a small sleep/backoff for rapid-failure cases to avoid busy retry behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

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

[Jeomon]

Thanks