fix(server): stop stacking a tool profile's visibility on every session
Release / test (full, windows-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.12) (push) Canceled after 0s
Release / build (push) Canceled after 0s
Release / publish-pypi (push) Canceled after 0s
Release / github-release (push) Canceled after 0s
Release / test (full, windows-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.12) (push) Canceled after 0s
Release / build (push) Canceled after 0s
Release / publish-pypi (push) Canceled after 0s
Release / github-release (push) Canceled after 0s
What the first Linux CI run of the 1.6 tree found. - fastmcp 3's enable()/disable() append a Visibility transform per call, and a tool lookup walks the transform stack as nested calls. The lifespan re-applies the tool profile on every client connection, so an in-process server grew two layers per session until every tool call, system_status included, raised RecursionError (CPython 3.11 counts C frames against the limit). The profile's transforms now replace the previous ones, as _apply_discovery_mode already did; a test switches the profile 600 times and then calls a tool. The local suite runs in half the time it took with the stack growing. - 21 fake-ActiveX tests build a real pywin32 VARIANT; they now skip without pywin32, like the suite's other VARIANT-dependent COM tests, and the Windows lane keeps running them. - A test spelled a path through a directory it never created, which only Windows normalises away; it creates the directory. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
17bf3a39ce
commit
cdb1063896
@@ -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
|
||||
|
||||
@@ -19,7 +19,7 @@ Live through COM on Windows, or headless through ezdxf anywhere — one typed co
|
||||
|
||||
</div>
|
||||
|
||||
> **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.
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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'"):
|
||||
|
||||
Reference in New Issue
Block a user