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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5
parent
e8e7e3c524
commit
df2ab91900
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user