From 1b0fe566f7009111e40b218bf6dc6b71c464d7b9 Mon Sep 17 00:00:00 2001 From: Nikolay Miroshnichenko Date: Sun, 28 Jun 2026 10:24:17 +0200 Subject: [PATCH] Fix infinite recursion in load_project MCP tool The server tool function `load_project` shadowed the imported project loader, so it called itself instead of the parser -> RecursionError on every real .knxproj. Alias the import as `load_project_file` and call that. Add a regression test (server.load_project on a bad path must not raise RecursionError) and a CHANGELOG entry. Found via end-to-end testing against real ETS5/ETS6 fixtures from XKNX/xknxproject. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 6 ++++++ nickol_knx_mcp/server.py | 4 ++-- tests/test_pipeline.py | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0e0276..4bd5506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **Critical:** `load_project` MCP tool recursed infinitely (`RecursionError`) on every + real `.knxproj` because the server tool function shadowed the imported project loader. + The tool now delegates correctly. Added a regression test that fails on recursion. + Found via end-to-end testing against real ETS5/ETS6 project files. + ## [0.1.0] — 2026-06-28 Initial public beta. diff --git a/nickol_knx_mcp/server.py b/nickol_knx_mcp/server.py index 5cb2a96..261a62c 100644 --- a/nickol_knx_mcp/server.py +++ b/nickol_knx_mcp/server.py @@ -15,7 +15,7 @@ from typing import Any, Optional from mcp.server.fastmcp import FastMCP -from .project import load_project, LoadedProject +from .project import load_project as load_project_file, LoadedProject from .analyze import validate_naming, detect_missing_status, detect_dpt_issues from .generate_ha import generate_ha_yaml from .generate_ets import generate_ets_csv, generate_ets_xml @@ -69,7 +69,7 @@ def load_project(path: str, password: Optional[str] = None, password: Project password, if the .knxproj is protected. language: Optional language code (e.g. 'de-DE', 'ru-RU'). """ - proj = load_project(path, password=password, language=language) + proj = load_project_file(path, password=password, language=language) _STATE["project"] = proj return { "loaded": True, diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 2b1060a..df8fd64 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -126,3 +126,21 @@ rep = build_report(proj) print(rep["summary"]) print("\n--- report head ---") print("\n".join(rep["markdown"].splitlines()[:30])) + +# --------------------------------------------------------------------------- # +# Regression: the MCP server tool `load_project` must delegate to the project +# loader, not call itself (a name-shadowing bug once caused infinite recursion). +# Calling it on a bad path must raise a normal error, never RecursionError. +# --------------------------------------------------------------------------- # +print("\n=== REGRESSION: server.load_project must not recurse ===") +import nickol_knx_mcp.server as _server +try: + _server.load_project("/nonexistent/__no_such__.knxproj") +except RecursionError as exc: # pragma: no cover + raise AssertionError( + "server.load_project recursed — load_project name-shadowing regression" + ) from exc +except Exception as exc: + print(f"OK: delegated, raised {type(exc).__name__} (expected, not RecursionError)") +else: + print("OK: delegated (no error)")