diff --git a/CHANGELOG.md b/CHANGELOG.md index 32a7d49..ea5bc65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -265,6 +265,21 @@ member. Spec: `docs/superpowers/specs/2026-09-24-v1.6-understand-design.md`. ### Fixed +- **Every tool call raised `RecursionError` after a few hundred in-process + sessions.** fastmcp 3's `enable()` / `disable()` append a Visibility + transform per call, and a tool lookup walks the whole transform stack as + nested calls. `_apply_tool_profile` runs in the lifespan, once per client + connection, so each session stacked two more layers. The first Linux CI run + of the 1.6 tree found it: on CPython 3.11, which counts C frames against the + recursion limit, `system_status` itself failed from about 90 % of the suite + on. The profile's transforms now replace the previous ones instead of + stacking on them, the idiom `_apply_discovery_mode` already used, and a test + switches the profile 600 times and then calls a tool. +- The same CI run failed 21 fake-ActiveX tests off Windows (they build a real + pywin32 VARIANT and now skip without pywin32, like the suite's other + VARIANT-dependent COM tests; the Windows lane runs them), and a test that + spelled a path through a directory it never created, which only Windows + normalises away. - **Found by the 1.6.0 README renders** (each with a test that fails on the old code): - The exterior dimension chains of `arch_dimension_chains` and diff --git a/README.md b/README.md index 14680bc..b714b3a 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ Live through COM on Windows, or headless through ezdxf anywhere — one typed co -> **v1.6 release snapshot:** 247 tools · 8 resources · 5 prompt templates · 4991 collected tests. +> **v1.6 release snapshot:** 247 tools · 8 resources · 5 prompt templates · 4992 collected tests. > 247 is the **registered** count; a default install advertises 242 over `tools/list`, > because `ENABLE_3D` is unset. `system_about` is the runtime authority. diff --git a/server.py b/server.py index d8288b1..d1bc491 100644 --- a/server.py +++ b/server.py @@ -1089,6 +1089,9 @@ PACK_TOOL_NAMES: dict[str, frozenset[str]] = { } _active_tool_profile: dict | None = None +#: The Visibility transforms the last _apply_tool_profile added, detached before +#: the next one is applied. +_profile_transforms: list = [] def _enabled_packs() -> tuple[set[str], list[str]]: @@ -1160,10 +1163,23 @@ async def _apply_tool_profile(profile: str | None = None) -> dict: registered = {tool.name for tool in await _registered_tools() if getattr(tool, "name", None)} enabled = _profile_enabled_names(selected, registered) disabled = sorted(registered - enabled) + # Replace the previous profile's visibility, never stack on it. fastmcp 3's + # enable()/disable() append a Visibility transform per call and a tool lookup + # walks the whole stack as nested calls; the lifespan runs this on every + # client connection, so an in-process server grew two layers per session + # until every tool call raised RecursionError (Linux CI, CPython 3.11). The + # same detach-first idiom as _apply_discovery_mode. + for transform in _profile_transforms: + try: + mcp._transforms.remove(transform) + except (AttributeError, ValueError) as exc: # pragma: no cover - layout change + log.debug("Could not detach a tool-profile transform: %s", exc) + before = list(mcp.transforms) if enabled: mcp.enable(names=set(enabled)) if disabled: mcp.disable(names=set(disabled)) + _profile_transforms[:] = [t for t in mcp.transforms if not any(t is b for b in before)] _active_tool_profile = { "profile": selected, "registered_count": len(registered), diff --git a/tests/test_com_backend.py b/tests/test_com_backend.py index f3de2cf..ebb60f2 100644 --- a/tests/test_com_backend.py +++ b/tests/test_com_backend.py @@ -10,6 +10,7 @@ Strategy: from __future__ import annotations +import importlib.util import math import sys from types import SimpleNamespace @@ -21,6 +22,13 @@ import pytest from backends.base import normalize_lineweight from backends.com_backend import ComBackend +#: A path that marshals a point or an object array builds a real pywin32 VARIANT, +#: and pywin32 exists only on Windows. The Windows lane runs these; elsewhere +#: they skip, like the suite's other VARIANT-dependent COM tests. +needs_pywin32 = pytest.mark.skipif( + importlib.util.find_spec("win32com") is None, reason="pywin32 not installed" +) + @pytest.mark.asyncio async def test_active_document_switch_clears_com_document_state(monkeypatch): @@ -1312,6 +1320,7 @@ def _square(x0, y0, size): @pytest.mark.asyncio +@needs_pywin32 async def test_a_chain_of_lines_becomes_one_closed_polyline_loop(island_seat): """A flattened bore is 180 line edges; one temporary polyline carries them in a single COM call and is deleted once the hatch has evaluated it.""" @@ -1327,6 +1336,7 @@ async def test_a_chain_of_lines_becomes_one_closed_polyline_loop(island_seat): @pytest.mark.asyncio +@needs_pywin32 async def test_arcs_stay_true_arcs_in_radians_and_clockwise_is_swapped(island_seat): """Curved-edge fidelity is the reason this tool exists: an arc edge becomes an AcDbArc, never a chord chain. ActiveX arcs run counter-clockwise, so a @@ -1355,6 +1365,7 @@ async def test_arcs_stay_true_arcs_in_radians_and_clockwise_is_swapped(island_se @pytest.mark.asyncio +@needs_pywin32 async def test_a_loop_autocad_refuses_is_taken_back_out(island_seat): """AutoCAD answers an open loop with E_FAIL (measured); the temporary entities are deleted and the call says nothing was added.""" diff --git a/tests/test_documents.py b/tests/test_documents.py index c5809e8..ddf8f5f 100644 --- a/tests/test_documents.py +++ b/tests/test_documents.py @@ -190,6 +190,9 @@ async def test_save_to_the_document_own_path_is_not_a_collision(backend, tmp_pat assert (await backend.drawing_save(str(path)))["ok"] is True await backend.entity_create_circle(0, 0, 3) # A spelling that differs from the key still names the same file. + # POSIX resolves sub/.. through the directory, so it must exist; Windows + # normalises the spelling before it looks. + (tmp_path / "sub").mkdir() spelled = str(tmp_path / "sub" / ".." / "self.dxf") assert (await backend.drawing_save_as(spelled))["ok"] is True assert len(await backend.document_list()) == 2 diff --git a/tests/test_refs_contract.py b/tests/test_refs_contract.py index 990e47a..00132e6 100644 --- a/tests/test_refs_contract.py +++ b/tests/test_refs_contract.py @@ -22,6 +22,11 @@ from backends.contracts.refs import ( ) from backends.ezdxf_backend import EzdxfBackend +#: A path that marshals a point or an object array builds a real pywin32 VARIANT, +#: and pywin32 exists only on Windows. The Windows lane runs these; elsewhere +#: they skip, like the suite's other VARIANT-dependent COM tests. +needs_pywin32 = pytest.mark.skipif(find_spec("win32com") is None, reason="pywin32 not installed") + TRACK_G_KEYS = ("dwg_write", "xref_live", "xlsx_write") @@ -385,6 +390,7 @@ class _ComDoc: @pytest.mark.asyncio +@needs_pywin32 async def test_com_attach_calls_the_measured_member_with_radians(monkeypatch, external): import math @@ -478,6 +484,7 @@ async def test_com_attach_refuses_a_plain_block_of_the_same_name_case_insensitiv @pytest.mark.asyncio +@needs_pywin32 async def test_com_attach_still_works_when_no_block_of_that_name_exists(monkeypatch, external): import backends.com_backend as cb @@ -489,6 +496,7 @@ async def test_com_attach_still_works_when_no_block_of_that_name_exists(monkeypa @pytest.mark.asyncio +@needs_pywin32 async def test_com_raster_calls_addraster_with_radians(monkeypatch, tmp_path): import math diff --git a/tests/test_tool_profiles.py b/tests/test_tool_profiles.py index 46b33b4..521bd1b 100644 --- a/tests/test_tool_profiles.py +++ b/tests/test_tool_profiles.py @@ -120,3 +120,25 @@ async def test_profile_switch_is_idempotent(): assert set(info["disabled_tools"]) == set(server.SOLID_TOOL_NAMES) lean_again = await server._apply_tool_profile("lean") assert lean_again["enabled_count"] == len(server.LEAN_TOOL_NAMES) + + +async def test_reapplying_a_profile_replaces_its_visibility_instead_of_stacking(monkeypatch): + # fastmcp 3 appends a Visibility transform on every enable/disable call, and a + # tool lookup walks the whole stack as nested calls. The lifespan re-applies + # the profile on every client connection, so the stack grew by two per + # session: after a few hundred in-process sessions every tool call raised + # RecursionError (Linux CI, CPython 3.11, which counts C frames too). + await server._apply_tool_profile("full") + depth = len(server.mcp.transforms) + for _ in range(300): + await server._apply_tool_profile("lean") + await server._apply_tool_profile("full") + assert len(server.mcp.transforms) == depth + + monkeypatch.setenv("AUTOCAD_MCP_BACKEND", "ezdxf") + monkeypatch.setattr(config.settings, "tool_profile", "lean") + async with Client(server.mcp) as client: + visible = {tool.name for tool in await client.list_tools()} + status = (await client.call_tool("system_status", {})).structured_content + assert visible == set(server.LEAN_TOOL_NAMES) + assert status["backend"] == "ezdxf" diff --git a/tests/test_track_a_hardening.py b/tests/test_track_a_hardening.py index 1b3916e..f759e5f 100644 --- a/tests/test_track_a_hardening.py +++ b/tests/test_track_a_hardening.py @@ -9,12 +9,20 @@ is dispatched at all). from __future__ import annotations +import importlib.util import types import pytest from backends.base import EntityInfo +#: A path that marshals a point or an object array builds a real pywin32 VARIANT, +#: and pywin32 exists only on Windows. The Windows lane runs these; elsewhere +#: they skip, like the suite's other VARIANT-dependent COM tests. +needs_pywin32 = pytest.mark.skipif( + importlib.util.find_spec("win32com") is None, reason="pywin32 not installed" +) + pytestmark = pytest.mark.asyncio @@ -251,6 +259,7 @@ async def test_a_non_string_block_name_is_a_type_error_on_both_placing_tools(bac await backend.block_insert("", 0, 0) +@needs_pywin32 async def test_com_block_ref_probes_the_block_table_before_insert_block(com): backend, document, space = com with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"): @@ -332,6 +341,7 @@ async def test_explode_refuses_a_non_insert_without_writing(backend): assert len(list(backend._msp())) == before +@needs_pywin32 async def test_com_explode_deletes_attdefs_adds_text_per_attrib_and_deletes_the_reference(com): backend, document, space = com attdef = _FakeObject("AcDbAttributeDefinition", "D1", Constant=False) @@ -395,6 +405,7 @@ async def test_com_explode_refuses_a_non_insert_before_any_call(com): assert space.calls == [] +@needs_pywin32 async def test_com_explode_adds_the_text_to_the_owner_block_not_the_active_layout(com): """ActiveX ``Explode()`` places the members in the owner space; the tag TEXT must land beside them, not in whatever tab is active.""" @@ -456,6 +467,7 @@ async def test_com_explode_refuses_a_nested_reference_before_explode(com): assert dispatched == [] and space.calls == [] and ref.deleted is False +@needs_pywin32 async def test_com_explode_carries_the_attrib_frame_and_style_onto_the_text(com): """The COM twin of the mirrored-reference test. ``AddText`` builds a +Z TEXT; an ATTRIB reflected to ``Normal (0, 0, -1)`` has an OCS ``Rotation`` @@ -541,6 +553,7 @@ async def test_com_explode_carries_the_attrib_frame_and_style_onto_the_text(com) } +@needs_pywin32 async def test_com_explode_skips_frame_members_the_attrib_does_not_expose(com): """A bare ATTRIB (the shape the earlier fakes use) still explodes: an absent optional member is skipped, never written as a guess, and a @@ -580,6 +593,7 @@ async def test_com_explode_skips_frame_members_the_attrib_does_not_expose(com): assert names == ["Rotation", "Alignment", "Layer"] +@needs_pywin32 async def test_com_explode_turns_a_constant_attdef_into_text_instead_of_deleting_it(com): """``GetAttributes()`` excludes constant attributes (``GetConstantAttributes()`` lists them) and ``Explode()`` returns each as an ``AcDbAttributeDefinition`` @@ -645,6 +659,7 @@ async def test_com_explode_turns_a_constant_attdef_into_text_instead_of_deleting ], "the constant ATTDEF's placement, angle and alignment ride onto its TEXT" +@needs_pywin32 async def test_com_explode_falls_back_to_the_constant_tag_list_when_constant_is_absent(com): """An object that does not expose ``Constant`` is still recognised through ``GetConstantAttributes()``; a value-less placeholder is still just deleted.""" @@ -937,6 +952,7 @@ async def test_explode_of_a_single_cell_grid_is_still_an_ordinary_insert(backend assert [(await backend.entity_get(h)).type for h in result["inserted_handles"]] == ["CIRCLE"] +@needs_pywin32 async def test_com_explode_turns_a_multi_line_attrib_into_mtext(com): """``TextString`` of a multi-line attribute is one flattened string; the content is ``MTextAttributeContent`` and the box ``MTextBoundaryWidth``. @@ -1018,6 +1034,7 @@ async def test_com_explode_turns_a_multi_line_attrib_into_mtext(com): assert ref.deleted is True +@needs_pywin32 async def test_com_explode_multi_line_attrib_takes_the_frame_before_the_angle(com): backend, document, space = com note = _FakeObject( @@ -1689,6 +1706,7 @@ class _SpaceFailingSecondAddText(_FakeSpace): return super().AddText(text, point, height) +@needs_pywin32 async def test_com_explode_undoes_a_burst_that_fails_after_explode(com): """ActiveX ``Explode()`` leaves the reference in place, so a failure after it (here ``AddText`` on the second attribute; a constant ATTDEF whose @@ -1794,6 +1812,7 @@ async def test_set_attributes_refuses_by_tag_and_leaves_the_value_alone(backend) assert await backend.block_get_attributes(ref.handle) == {"TAG": "7"} +@needs_pywin32 async def test_com_insert_refuses_before_insert_block_and_writes_numbers_as_text(com): backend, document, space = com space.attributes = {"TAG": ""} @@ -1876,6 +1895,7 @@ async def test_define_on_an_existing_layer_reports_nothing_created(backend): assert result["layers_created"] == [], "layer 0 always exists" +@needs_pywin32 async def test_com_define_probes_layers_before_blocks_add(com): backend, document, space = com with pytest.raises(ValueError, match="'NOT_A_LAYER' do not exist"): @@ -1921,6 +1941,7 @@ async def test_define_treats_an_existing_layer_case_insensitively(backend): assert result["layers_created"] == [] +@needs_pywin32 async def test_com_define_creates_a_case_folded_layer_once(com): backend, document, space = com with pytest.raises(ValueError, match=r"layer\(s\) 'Alpha' do not exist"): @@ -1933,6 +1954,7 @@ async def test_com_define_creates_a_case_folded_layer_once(com): assert [name for name, _ in document.Blocks.blocks["PID_CASE"].calls] == ["AddLine", "AddLine"] +@needs_pywin32 async def test_com_define_treats_an_existing_layer_case_insensitively(com): backend, document, space = com document.Layers.Add("PID-EQUIP") @@ -1941,6 +1963,7 @@ async def test_com_define_treats_an_existing_layer_case_insensitively(com): assert result["layers_created"] == [] and document.Layers.added == ["PID-EQUIP"] +@needs_pywin32 async def test_com_define_refuses_a_bad_base_point_before_blocks_add(com): backend, document, space = com with pytest.raises(TypeError, match="'base_y'"):