fix(blocks): entity_create_block_ref refuses an undefined or layout block before writing, on both engines

Track A's task-1 gate covered block_insert only; the sibling tool still
returned a handle for a typo'd name that doc.audit() later deleted, and
both tools accepted *Model_Space (a reference cycle). One gate per engine
(_require_block / _require_block_defined) is now shared by both placing
tools: non-string name -> TypeError, unknown -> ValueError naming it,
layout block -> ValueError, all before add_blockref / InsertBlock.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd

backends/com_backend.py

@@ -193,6 +193,30 @@ def _msp():
         return doc.ModelSpace
 
 
+def _require_block_defined(doc, name):
+    """``doc.Blocks.Item(name)``, or a ``ValueError`` before ``InsertBlock`` runs.
+
+    ``InsertBlock`` with an unknown name raises a bare COM error after the call
+    has already been dispatched; refusing here names the block, costs one
+    ``Blocks.Item`` probe and matches the headless engine's message. A layout
+    block (``IsLayout``) cannot be inserted into itself — same refusal by name.
+    Shared by ``block_insert`` and ``entity_create_block_ref``.
+    """
+    if not isinstance(name, str) or not name.strip():
+        raise TypeError("block name must be a non-empty string")
+    try:
+        block = doc.Blocks.Item(name)
+    except Exception as exc:
+        raise ValueError(f"block {name!r} is not defined") from exc
+    try:
+        is_layout = bool(block.IsLayout)
+    except Exception:
+        is_layout = False
+    if is_layout:
+        raise ValueError(f"block {name!r} is a layout block and cannot be inserted")
+    return block
+
+
 _BUILTIN_LINETYPES = {"continuous", "bylayer", "byblock"}
 
 
@@ -2479,6 +2503,7 @@ class ComBackend(AutoCADBackend):
         layer=None,
     ) -> EntityInfo:
         def _sync():
+            _require_block_defined(_acad_doc(), name)
             mspace = _msp()
             ref = mspace.InsertBlock(
                 _apoint(x, y),
@@ -3248,6 +3273,7 @@ class ComBackend(AutoCADBackend):
         layer=None,
     ) -> EntityInfo:
         def _sync():
+            _require_block_defined(_acad_doc(), name)
             mspace = _msp()
             ref = mspace.InsertBlock(
                 _apoint(x, y),

backends/ezdxf_backend.py

@@ -1087,6 +1087,28 @@ class EzdxfBackend(AutoCADBackend):
             raise RuntimeError("No document open. Call drawing_new() or drawing_open() first.")
         return self._doc
 
+    def _require_block(self, name):
+        """The block definition ``name`` names, or a refusal before any write.
+
+        ``add_blockref`` never checks the name and ``add_auto_attribs`` skips
+        autofill when ``Insert.block()`` resolves to None, so a typo'd name
+        used to yield a dangling INSERT that dropped its attribute values and
+        that ``doc.audit()`` later deletes. A layout block (``*Model_Space``,
+        ``*Paper_Space``) is in ``doc.blocks`` too, but inserting one is the
+        reference cycle AutoCAD's INSERT refuses, so it is refused here by
+        name. Shared by ``block_insert`` and ``entity_create_block_ref`` so the
+        two tools that place a block reference agree on what a typo does.
+        """
+        doc = self._require_doc()
+        if not isinstance(name, str) or not name.strip():
+            raise TypeError("block name must be a non-empty string")
+        if name not in doc.blocks:
+            raise ValueError(f"block {name!r} is not defined")
+        blk = doc.blocks.get(name)
+        if blk.block_record.is_any_layout:
+            raise ValueError(f"block {name!r} is a layout block and cannot be inserted")
+        return blk
+
     def _msp(self):
         """The space new geometry goes into — model space, or the current layout.
 
@@ -3002,6 +3024,7 @@ class EzdxfBackend(AutoCADBackend):
     ) -> EntityInfo:
         def _sync():
             msp = self._msp()
+            self._require_block(name)
             ent = msp.add_blockref(
                 name,
                 (float(x), float(y)),
@@ -4485,13 +4508,7 @@ class EzdxfBackend(AutoCADBackend):
             }
             if layer:
                 attribs["layer"] = layer
-            # `add_blockref` never checks the name, and `add_auto_attribs`
-            # silently skips autofill when `Insert.block()` resolves to None,
-            # so a typo'd name used to yield a dangling INSERT that dropped its
-            # attribute values and that `doc.audit()` later deletes. Refuse
-            # before writing, as COM's `InsertBlock` does for an unknown name.
-            if name not in self._doc.blocks:
-                raise ValueError(f"block {name!r} is not defined")
+            self._require_block(name)
             ref = msp.add_blockref(name, (float(x), float(y)), dxfattribs=attribs)
             if attributes:
                 # `add_auto_blockref` wraps the INSERT in an anonymous *U block,

server.py

@@ -1656,7 +1656,14 @@ async def entity_create_block_ref(
     layer: Annotated[str | None, "Layer name"] = None,
     ctx: Context = None,
 ) -> dict:
-    """Insert a block reference (instance of an existing block definition)."""
+    """Insert a block reference (instance of an existing block definition).
+
+    Refused before any write, on both engines: a name that is not a block
+    definition in this drawing (`block 'X' is not defined` — check
+    `block_list`), a layout block (`*Model_Space` / `*Paper_Space`, a
+    reference cycle) and a non-string name. The same gate as `block_insert`;
+    this tool is the no-attributes form.
+    """
     await ctx.debug(f"Inserting block '{name}' at ({x},{y})")
     result = await _backend(ctx).entity_create_block_ref(
         name, x, y, scale_x, scale_y, rotation, layer

tests/test_track_a_hardening.py

@@ -0,0 +1,220 @@
+"""Track A hardening (v1.6 track E, wave 0): the block-tool defects the Track A
+review found, fixed on both engines.
+
+Headless tests run against a real ezdxf document; the live engine runs against
+a fake ActiveX surface that records every call, so the tests assert the call
+shape (what is dispatched, in which order, and — for a refusal — that nothing
+is dispatched at all).
+"""
+
+from __future__ import annotations
+
+import types
+
+import pytest
+
+from backends.base import EntityInfo
+
+pytestmark = pytest.mark.asyncio
+
+
+def _define_tagged_block(backend, name: str = "TB") -> None:
+    blk = backend._doc.blocks.new(name=name)
+    blk.add_line((0, 0), (10, 0))
+    blk.add_attdef("TAG", insert=(0, 3), text="", dxfattribs={"height": 2.5})
+
+
+# ── fake ActiveX surface ──────────────────────────────────────────────────────
+
+
+class _FakeObject:
+    """An ActiveX object with only the members it is given; property writes are recorded."""
+
+    def __init__(self, object_name: str, handle: str, **members):
+        object.__setattr__(self, "ObjectName", object_name)
+        object.__setattr__(self, "Handle", handle)
+        object.__setattr__(self, "writes", [])
+        object.__setattr__(self, "deleted", False)
+        for name, value in members.items():
+            object.__setattr__(self, name, value)
+
+    def __setattr__(self, name, value):
+        self.writes.append((name, value))
+        object.__setattr__(self, name, value)
+
+    def __getattr__(self, name):
+        raise AttributeError(name)
+
+    def Delete(self):
+        object.__setattr__(self, "deleted", True)
+
+
+class _FakeBlock:
+    def __init__(self, name: str, is_layout: bool = False):
+        self.Name = name
+        self.IsLayout = is_layout
+        self.Count = 0
+        self.calls: list[tuple[str, tuple]] = []
+
+    def __getattr__(self, attr):
+        if attr.startswith("Add"):
+
+            def _call(*args):
+                self.calls.append((attr, args))
+                return _FakeObject("AcDbEntity", f"M{len(self.calls)}")
+
+            return _call
+        raise AttributeError(attr)
+
+
+class _FakeBlocks:
+    def __init__(self, *names: str):
+        self.blocks: dict[str, _FakeBlock] = {
+            "*Model_Space": _FakeBlock("*Model_Space", is_layout=True),
+        }
+        for name in names:
+            self.blocks[name] = _FakeBlock(name)
+
+    def Add(self, base_point, name):
+        self.blocks[name] = _FakeBlock(name)
+        self.blocks[name].base_point = tuple(base_point.value)
+        return self.blocks[name]
+
+    def Item(self, name):
+        if name not in self.blocks:
+            raise RuntimeError(f"no block {name}")
+        return self.blocks[name]
+
+
+class _FakeLayers:
+    def __init__(self, *names: str):
+        self.names = {"0", *names}
+        self.added: list[str] = []
+
+    def Item(self, name):
+        if name not in self.names:
+            raise RuntimeError(f"no layer {name}")
+        return types.SimpleNamespace(Name=name)
+
+    def Add(self, name):
+        self.names.add(name)
+        self.added.append(name)
+        return types.SimpleNamespace(Name=name)
+
+
+class _FakeSpace:
+    """The active layout block: records InsertBlock / AddText and hands back styled objects."""
+
+    def __init__(self, attributes: dict[str, str] | None = None):
+        self.calls: list[tuple[str, tuple]] = []
+        self.attributes = attributes or {}
+        self.inserted: list[_FakeObject] = []
+
+    def InsertBlock(self, point, name, sx, sy, sz, rotation):
+        self.calls.append(("InsertBlock", (tuple(point.value), name, sx, sy, sz, rotation)))
+        attrs = tuple(
+            _FakeObject("AcDbAttribute", f"A{i}", TagString=tag, TextString=value)
+            for i, (tag, value) in enumerate(self.attributes.items())
+        )
+        ref = _FakeObject(
+            "AcDbBlockReference", f"R{len(self.inserted)}", Name=name, GetAttributes=lambda: attrs
+        )
+        self.inserted.append(ref)
+        return ref
+
+    def AddText(self, text, point, height):
+        self.calls.append(("AddText", (text, tuple(point.value), height)))
+        return _FakeObject("AcDbText", f"T{len(self.calls)}")
+
+
+@pytest.fixture
+def com(monkeypatch):
+    """A ComBackend over a recording fake: ``document`` and ``space`` are inspectable."""
+    from backends import com_backend as module
+
+    document = types.SimpleNamespace(Blocks=_FakeBlocks("TB"), Layers=_FakeLayers(), objects={})
+    document.HandleToObject = lambda handle: document.objects[handle]
+    space = _FakeSpace()
+    monkeypatch.setattr(module, "_acad_doc", lambda: document)
+    monkeypatch.setattr(module, "_msp", lambda: space)
+    monkeypatch.setattr(module, "_regen", lambda: None)
+    # The real converter reads a dozen ActiveX members (bounding box, block
+    # geometry, ...); the fixture only needs the handle and the name back.
+    monkeypatch.setattr(
+        module,
+        "_entity_info",
+        lambda ent: EntityInfo(
+            handle=ent.Handle,
+            type="INSERT",
+            layer="0",
+            color=256,
+            linetype="ByLayer",
+            visible=True,
+            properties={"block_name": ent.Name},
+        ),
+    )
+    backend = module.ComBackend()
+
+    async def _run_inline(func, *args, **kwargs):
+        return func(*args, **kwargs)
+
+    monkeypatch.setattr(backend, "_run", _run_inline)
+    return backend, document, space
+
+
+# ── Task 1: entity_create_block_ref refuses an undefined block ────────────────
+
+
+async def test_block_ref_of_an_undefined_block_is_refused_before_writing(backend):
+    before = len(list(backend._msp()))
+    with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"):
+        await backend.entity_create_block_ref("NO_SUCH_BLOCK", 1, 2)
+    assert len(list(backend._msp())) == before, "a refused insert must not write"
+    assert "NO_SUCH_BLOCK" not in backend._doc.blocks
+
+
+async def test_block_ref_of_a_defined_block_still_inserts(backend):
+    _define_tagged_block(backend)
+    ref = await backend.entity_create_block_ref("TB", 5, 5, rotation=90)
+    info = await backend.entity_get(ref.handle)
+    assert info.type == "INSERT" and info.properties["block_name"] == "TB"
+    assert info.properties["rotation_deg"] == pytest.approx(90.0)
+
+
+@pytest.mark.parametrize("name", ["*Model_Space", "*Paper_Space"])
+async def test_layout_blocks_are_refused_by_both_placing_tools(backend, name):
+    with pytest.raises(ValueError, match="layout block"):
+        await backend.entity_create_block_ref(name, 0, 0)
+    with pytest.raises(ValueError, match="layout block"):
+        await backend.block_insert(name, 0, 0)
+    assert backend._doc.audit().errors == [], "nothing was written, nothing to fix"
+
+
+async def test_a_non_string_block_name_is_a_type_error_on_both_placing_tools(backend):
+    with pytest.raises(TypeError, match="non-empty string"):
+        await backend.entity_create_block_ref(None, 0, 0)
+    with pytest.raises(TypeError, match="non-empty string"):
+        await backend.block_insert("", 0, 0)
+
+
+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"):
+        await backend.entity_create_block_ref("NO_SUCH_BLOCK", 1, 2)
+    assert space.calls == [], "InsertBlock must not be dispatched for an unknown name"
+    with pytest.raises(ValueError, match="layout block"):
+        await backend.entity_create_block_ref("*Model_Space", 0, 0)
+    assert space.calls == []
+
+    ref = await backend.entity_create_block_ref("TB", 1, 2, 2.0, 2.0, 90.0)
+    assert ref.properties["block_name"] == "TB"
+    assert space.calls == [
+        ("InsertBlock", ((1.0, 2.0, 0.0), "TB", 2.0, 2.0, 1.0, pytest.approx(1.5708, abs=1e-4)))
+    ]
+
+
+async def test_com_block_insert_shares_the_same_gate(com):
+    backend, document, space = com
+    with pytest.raises(ValueError, match="'NO_SUCH_BLOCK' is not defined"):
+        await backend.block_insert("NO_SUCH_BLOCK", 1, 2, attributes={"TAG": "P-1"})
+    assert space.calls == []