From df2ab91900905fbcf9db7b89479d7b5f46051180 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Wed, 16 Sep 2026 21:48:09 +0300 Subject: [PATCH] fix(task-14): return the plain point list from COM system_set_variable, not the VARIANT The tuple/list branch put the win32com VARIANT built for SetVariable into the returned dict. The system_set_variable tool returns that dict verbatim under output_schema {"type": "object"}; pydantic cannot serialise a VARIANT, FastMCP drops the structured content and the client sees "outputSchema defined but no structured output returned" -- after AutoCAD had already applied the write, so every raw point-variable write (LIMMIN, LIMMAX, GRIDUNIT, SNAPUNIT, INSBASE) was a retry/undo trap on the live engine while the audit log said TOOL OK. The VARIANT is now only the SetVariable argument; "value" carries the coerced Python list. The fake-ActiveX test asserts the reply, and a new Client(server.mcp) test drives the tool through the output schema so the serialisation path is covered. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- backends/com_backend.py | 11 +++++++++-- tests/test_settings_facade.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/backends/com_backend.py b/backends/com_backend.py index 8fb117e..7436dec 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -4003,6 +4003,12 @@ class ComBackend(AutoCADBackend): # AutoCAD rejects e.g. a string "0" for the integer OSMODE. Probe the # current value's type and convert to match; pass through on failure. coerced = value + # `payload` is what SetVariable receives. It differs from `coerced` + # only for point variables, whose VARIANT wrapper must never reach + # the returned dict: pydantic cannot serialise + # win32com.client.VARIANT, so the tool would report failure *after* + # AutoCAD had already applied the write. + payload = value try: current = app.GetVariable(name) if isinstance(current, bool): @@ -4017,10 +4023,11 @@ class ComBackend(AutoCADBackend): # a bare Python tuple is marshalled as VT_VARIANT and rejected. coords = [float(v) for v in value] coords += [0.0] * (len(current) - len(coords)) - coerced = _av(coords[: len(current)]) + coerced = coords[: len(current)] + payload = _av(coerced) if isinstance(current, (tuple, list)) else coerced except Exception as exc: log.debug("set_variable type probe for %s failed: %s", name, exc) - app.SetVariable(name, coerced) + app.SetVariable(name, payload) return {"ok": True, "variable": name, "value": coerced} return await self._run(_sync) diff --git a/tests/test_settings_facade.py b/tests/test_settings_facade.py index 2a8b8b6..3f6cde9 100644 --- a/tests/test_settings_facade.py +++ b/tests/test_settings_facade.py @@ -15,6 +15,7 @@ Measured before writing (ezdxf 1.4.4, 2026-09-16): from __future__ import annotations +import json import math import pytest @@ -477,10 +478,36 @@ async def test_com_point_variables_travel_as_variant_double_arrays(com_backend): real_set(name, value) app.SetVariable = _capture - await backend.system_set_variable("LIMMAX", (300, 200)) + res = await backend.system_set_variable("LIMMAX", (300, 200)) variant = seen["LIMMAX"] assert variant.varianttype == pythoncom.VT_ARRAY | pythoncom.VT_R8 assert tuple(variant.value) == (300.0, 200.0) + # The VARIANT is an ActiveX argument only. The reply carries the plain + # Python list -- a VARIANT in the dict cannot be serialised by pydantic, + # and the tool would then report failure after AutoCAD applied the write. + assert res == {"ok": True, "variable": "LIMMAX", "value": [300.0, 200.0]} + json.dumps(res) + + +async def test_com_point_variable_reply_survives_the_mcp_output_schema(com_backend, monkeypatch): + """Through the real tool: `system_set_variable` returns its dict verbatim + under `output_schema={"type": "object"}`, so anything pydantic cannot + serialise turns into `ToolError: outputSchema defined but no structured + output returned` *after* `SetVariable` has landed -- a retry/undo trap.""" + from fastmcp import Client + + import server + + backend, app = com_backend + + async def _fake_make_backend(): + return backend + + monkeypatch.setattr(server, "_make_backend", _fake_make_backend) + async with Client(server.mcp) as client: + res = await client.call_tool("system_set_variable", {"name": "LIMMAX", "value": [300, 200]}) + assert res.data == {"ok": True, "variable": "LIMMAX", "value": [300.0, 200.0]} + assert app.store["LIMMAX"] == (300.0, 200.0) async def test_com_refuses_before_touching_activex(com_backend):