fix(correctness-bundle): I16 correctness fixes + dead code removal + ruff clean
N1: COM hatch non-associative (AddHatch associativity=False) N2: ezdxf refuses DWG saves with clear error; server.py derives fmt from extension R19: dimension_aligned dist→distance; dimension_angular correct add_angular_dim_2l args (base/line1/line2/location) R9: polar array 360° no-duplicate (divisor=n when fill_angle%360<ε) R13: entity_offset side_x/side_y honored in ezdxf via cross-product (LINE) and distance-to-center (CIRCLE) R18: drawing_new bootstrap failure surfaces status="degraded" instead of swallowing silently Dead code: engineering/section.py, gear.generate_tooth_profile, CommandResult, _ACI_NAMES, set_layer_active, ensure_linetypes, ComBackend._batch_mode instance attr, engineering/__init__ build-seam comment Ruff: 38 auto-fixed + 5 manual → codebase is now ruff-clean; CI lint is now blocking 184 tests pass.
This commit is contained in:
Umutcan Edizsalan
@@ -0,0 +1,209 @@
|
||||
The lineweight truncation bug is confirmed devastating: ENGINEERING_LAYERS uses floats like 0.50, 0.25, 0.18, 0.13, 0.05 mm, and `layer_create` does `int(lineweight)` (ezdxf_backend.py:1034), so every one floors to 0. Counts confirmed: 109 tools, 6 resources, 5 prompts; LICENSE/benchmarks/ci.yml all absent. All load-bearing claims verified against source. I have everything needed to synthesize.
|
||||
|
||||
# AutoCAD MCP Pro — Understanding & Research Dossier
|
||||
|
||||
## 1. Architecture at a Glance
|
||||
|
||||
AutoCAD MCP Pro is a **single-file FastMCP 3.0 server** (`server.py`, 2881 lines) exposing **109 `@mcp.tool` registrations, 6 `@mcp.resource` endpoints, 5 `@mcp.prompt` templates** (verified by grep, not the docs' "87"). It uses a **strategy-pattern dual backend** behind one ABC (`backends/base.py`, ~73 abstract methods + 5 dataclasses): a **live COM backend** (`com_backend.py`, pywin32, single STA-thread `ThreadPoolExecutor`) and a **headless ezdxf backend** (`ezdxf_backend.py`, file-based DXF, all sync work via `asyncio.to_thread` under one `asyncio.Lock`). Backend selection (`server.py::_make_backend`, server.py:69): env `AUTOCAD_MCP_BACKEND`; Windows `auto`/`com` tries COM then falls back to ezdxf; non-Windows always ezdxf.
|
||||
|
||||
Request path: `ErrorHandlingMiddleware → AuditMiddleware → TimingMiddleware → LoggingMiddleware` (server.py:197-200) → tool fn → `_backend(ctx)` (server.py:207, raises `ToolError` if backend None) → identically-named backend method → `_dc()` dataclass→dict serialization (server.py:216). Lifespan (`autocad_lifespan`, server.py:107) builds the backend singleton into `ctx.lifespan_context["backend"]`.
|
||||
|
||||
A **deterministic engineering layer** (`engineering/` package) backs the "premium" workflow: parametric gear/keyway/titleblock primitives, a closed-enum `drawing_critique`, an 8-step `DrawingValidator`, ISO layer/linetype scaffolds, and a `PlanSpec` plan-before-draw contract. **The central architectural fact: the premium workflow is fully implemented only on ezdxf; the COM backend stubs all 8 premium meta-tools with `NotImplementedError` (com_backend.py:1637-1660, verified) — yet COM is the default Windows backend.**
|
||||
|
||||
---
|
||||
|
||||
## 2. Per-Subsystem Summary
|
||||
|
||||
**`server.py` (transport + tool registry).** Thin async wrappers over the backend singleton, grouped into 13 sections (1–12 + sub-sections 8b/8c/8d batch/template/validation + 13 premium). Owns server-side composition tools (`entity_delete_many` server.py:1065, `layer_isolate` :1216, `block_find_references` :1362, `analysis_layer_stats` :1506, `entity_batch_*` :1538/:1590, `template_*` :1695/:1730, `validation_check` :1749). Entry point manually parses `--transport/--port/--host` (server.py:2866) and refuses non-loopback HTTP unless `ALLOW_REMOTE_HTTP` + `MCP_AUTH_TOKEN` (`_validate_http_bind`).
|
||||
|
||||
**`backends/base.py` (the contract).** `AutoCADBackend` ABC: ~73 `@abstractmethod`, 2 concrete (`set_layer_active`, `ensure_linetypes`), 5 dataclasses (`EntityInfo/LayerInfo/BlockInfo/DrawingInfo/CommandResult`), 3 helpers (`shoelace_area`, `deg2rad`, `rad2deg`). Both backends define a body for every abstract method (no structural gaps); divergences are behavioral.
|
||||
|
||||
**`backends/ezdxf_backend.py` (headless).** Full create/modify/query/analysis/dimension/block/layer + premium meta-tools + corner ops. Transactions = full DXF snapshot to tempfile on `_undo_stack` (trimmed to `max_undo_stack=5`, O(drawing-size) per `transaction_begin`). View ops and `system_run_command/run_lisp` are documented no-ops. Screenshot/PDF via matplotlib.
|
||||
|
||||
**`backends/com_backend.py` (live AutoCAD).** All COM through one STA worker with per-call timeout (`com_call_timeout`, default 60s) and executor-rebuild on timeout/fatal HRESULT. Corner ops via `_safe_send_command` (CMDACTIVE-polling). Win32 `PrintWindow` screenshots. **8 premium meta-tools + `block_create_from_entities` are stubs.**
|
||||
|
||||
**`engineering/` (deterministic CAD).** `gear.py` (involute spur/helical outlines), `keyway.py` (DIN 6885), `titleblock.py` (ISO 7200 A3), `layers.py` (mech/pid/iso13567 sets), `critique.py` (6 focuses), `validator.py` (8-step gate), `plan_spec.py` (PlanSpec/Issue + ISO-128 lineweight set). `section.py` (`cutting_plane_line`, `apply_section_hatch`) and `gear.generate_tooth_profile` are **dead code** (exported, zero callers/tests).
|
||||
|
||||
**`tests/` (184 functions, 14 files).** One ezdxf-only `backend` fixture (`conftest.py`). COM backend, server→sanitizer wiring, and `section.py` have **zero coverage**.
|
||||
|
||||
---
|
||||
|
||||
## 3. Complete MCP Tool Inventory (grouped; ✅ real / ⚠️ partial / ❌ stub-or-missing)
|
||||
|
||||
> All 109 tools are **registered** in `server.py` and have **real ezdxf implementations**. "Status" below reflects the **COM backend** (the Windows default) plus correctness caveats. Nothing is purely aspirational/unregistered.
|
||||
|
||||
| § | Tools | ezdxf | COM | Notes |
|
||||
|---|---|---|---|---|
|
||||
| 1 Drawing (12, doc says 11) | info, new, open, save, save_as, export_dxf, export_pdf, purge, audit, close, undo, redo | ✅ (redo ❌ no-op ezdxf_backend.py:487) | ✅ | `drawing_redo` inert on ezdxf; header says 11 but 12 registered (drawing_redo server.py:426) |
|
||||
| 2 Entity Create (13) | line, circle, arc, polyline, rectangle(→polyline), text, mtext, hatch, spline, ellipse, point, block_ref | ✅ | ✅ | mtext has no rotation param on ezdxf (:578); hatch pattern-only single loop |
|
||||
| 3 Dimensions (5) | linear, aligned, angular, radius, diameter | ⚠️ aligned/angular **xfail** (tests/test_dimensions.py:16); angular ignores tx/ty + fixed dist=10 (ezdxf:705) | ✅ | No dimstyle/tolerance support either backend |
|
||||
| 4 Modify (14) | move, copy, rotate, scale, mirror, offset, trim, extend, fillet, chamfer, delete, array_rectangular, array_polar, set_properties | ⚠️ offset ignores side (:868); corner ops LINE+LINE only (:1535-1750); polar 360° dup (:946) | ⚠️ offset ignores side (:860); corner ops via SendCommand | |
|
||||
| 5 Query (3) | entity_get, entity_list, entity_delete_many | ✅ | ✅ | COM entity_list = O(n) per-item COM reads (:967) |
|
||||
| 6 Layer (14) | list, create, delete, set_current, modify, freeze, thaw, lock, unlock, hide, show, isolate, linetype_list, linetype_load | ⚠️ **lineweight int() truncation** (:1034) | ✅ | See RISK-2 |
|
||||
| 7 Block (7) | list, insert, explode, get_attributes, set_attributes, create_from_entities, find_references | ✅ | ⚠️ create_from_entities ❌ stub (:1253) | BlockInfo.description never populated |
|
||||
| 8 Analysis (8) | stats, find_in_region, measure_distance, measure_area, bounding_box, select_by_layer, select_by_type, layer_stats | ⚠️ region fallback insert/start only (:1330) | ✅ | hand-rolled loops vs ezdxf query/groupby |
|
||||
| 8b Batch (2) | entity_batch_create, entity_batch_modify | ✅ | ✅ | |
|
||||
| 8c Template (2) | template_apply_layers, template_list | ✅ | ✅ | Duplicates `_LAYER_TEMPLATES`/prompt layer sets |
|
||||
| 8d Validation (1) | validation_check | ✅ | ✅ | |
|
||||
| 9 View (4, doc says 5) | zoom_extents, zoom_window, screenshot, zoom_and_screenshot | ⚠️ zoom = no-op (:1394) | ✅ | Header says 5 |
|
||||
| 10 Transactions (3) | begin, commit, rollback | ✅ (commit file-I/O outside lock :1452) | ⚠️ flag-leak on error (:1402) | |
|
||||
| 11 System (6) | status, get_variable, set_variable, run_command, run_lisp, about | ⚠️ run_command/lisp no-op | ⚠️ run_lisp result always 'nil' (:1487); about catalog drifted | |
|
||||
| 12 Engineering (7) | gear_draw_helical/spur/section_aa, keyway_draw_keyed_bore/section, titleblock_apply_iso_a3, drawing_finalize | ✅ | ✅ (use generic entity_create_*) | Validator/titleblock import-guarded → None risk |
|
||||
| 13 Premium (8) | drawing_plan, drawing_critique, point_from_snap, construction_xline, construction_clear, drawing_apply_iso_layers, dimension_auto, entity_select_smart | ✅ | ❌ **all NotImplementedError** (:1637-1660) | **The documented-mandatory workflow is dead on COM** |
|
||||
|
||||
**Documented-but-missing entirely:** none. **Documented-as-working but stubbed on default backend:** the 8 premium tools (§13) + `block_create_from_entities` on COM.
|
||||
|
||||
---
|
||||
|
||||
## 4. Cross-Cutting RISK Register (merged, deduped, severity-sorted)
|
||||
|
||||
| # | Sev | Risk | Location | Evidence |
|
||||
|---|---|---|---|---|
|
||||
| R1 | **CRITICAL** | All 8 premium meta-tools `raise NotImplementedError` on COM (the Windows default). The "non-negotiable for production" workflow (`drawing_plan→apply_iso_layers→critique→finalize`) 500s on the primary platform. | com_backend.py:1637-1660 | **Verified verbatim**; ezdxf impls real (ezdxf_backend.py:1753-2050) |
|
||||
| R2 | **CRITICAL** | `layer_create` does `int(lineweight)` so every sub-1.0 ISO mm value (0.50/0.25/0.18/0.13/0.05) floors to **0**. The entire ISO-128 lineweight discipline is silently nulled; `drawing_apply_iso_layers`/`drawing_new` bootstrap produce flat 0-weight layers. | ezdxf_backend.py:1034; values engineering/layers.py:16-26 | **Both verified.** ezdxf wants integer hundredths (50=0.50mm), not raw mm |
|
||||
| R3 | **HIGH** | `_check_iso128` is a permanent no-op: `lw <= 0: continue` (critique.py:50) skips every layer made 0 by R2. The advertised ISO gate can never fire. Comment "the layer dataclass already normalises to mm" (critique.py:53) is **false** (ezdxf_backend.py:189 stores raw `float(lw)`). | engineering/critique.py:37-64 | **Verified verbatim** |
|
||||
| R4 | **HIGH** | `_check_dim_overlap` reads `properties['text_position'/'defpoint'/'insertion_point']`, but the ezdxf DIM extractor sets only `dim_type` (ezdxf_backend.py:166-168) → `dim_points` always empty → check is a silent no-op. | engineering/critique.py:194-197 | — |
|
||||
| R5 | **HIGH** | COM timeout path (`stuck.shutdown(wait=False)`) cannot kill the STA worker still blocked in SendCommand → **permanently leaks one CoInitialize'd thread + COM proxy per timeout**; no `CoUninitialize` anywhere. | com_backend.py:415-433, 68-71 | — |
|
||||
| R6 | **HIGH** | Zero COM-backend test coverage (~80 methods incl. UndoMark transactions, SendCommand corner ops, CMDACTIVE guard). Passing ezdxf transaction tests give **false confidence** for the semantically different COM model. | tests/ (no ComBackend import); conftest.py:8-15 | — |
|
||||
| R7 | **HIGH** | Server→sanitizer wiring untested: `test_security.py` tests `sanitize_command/lisp/validate_path` in isolation; no integration test proves `system_run_command/lisp` actually call them. A refactor dropping the call (server.py:2035/2052) passes all security tests while raw commands reach SendCommand. | server.py:2035,2052; tests/test_security.py | — |
|
||||
| R8 | **HIGH** | LISP-aliasing bypass (`(setq f command)(f "ERASE")`) is claimed-defended in a comment (security.py:138-140) but has **no adversarial test**; UNC/`~`/symlink path escapes in `validate_path` also untested. | security.py:138-179; test_security.py:177-231 | — |
|
||||
| R9 | **MED** | `entity_array_polar` full-360° fill double-places: `step=radians(360)/(count-1)` puts the last copy at 0°==original → duplicate. Hits the common bolt-circle/gear pattern the engineering layer relies on. | ezdxf_backend.py:946-947 | — |
|
||||
| R10 | **MED** | `drawing_finalize` calls `drawing_save_as(str(validated))` with no `fmt`. ezdxf default `fmt='dxf'` (ezdxf:345) vs COM `fmt='dwg'` (com:515) vs save_as tool default `'dwg'` (server:336) → 3-way inconsistency; format backend-dependent, not path-derived. | server.py:2299 | **Verified** (omits fmt) |
|
||||
| R11 | **MED** | `drawing_save_as(fmt='dwg')` on ezdxf writes DXF bytes to a `.dwg` path and returns `{'format':'dwg'}` — silent wrong-format file. ezdxf cannot write DWG; should error or use `odafc`. | ezdxf_backend.py:345-352 | — |
|
||||
| R12 | **MED** | `dimension_angular` ignores tx/ty, fixed `distance=10.0` → arc radius wrong for large/small parts; breaks COM parity. | ezdxf_backend.py:705-722 | — |
|
||||
| R13 | **MED** | `entity_offset` ignores `side_x/side_y` (both backends); only `distance` sign flips side; ARC/LWPOLYLINE/ELLIPSE raise "not supported". | ezdxf:868-908; com:860-874 | — |
|
||||
| R14 | **MED** | Engineering titleblock/validator symbols import-guarded to `None` (engineering/__init__.py:40-51); if module truly fails to import, tools raise opaque "NoneType not callable" instead of clear ToolError. | server.py:2266,2294 | — |
|
||||
| R15 | **MED** | `system_about` static `tool_groups` catalog drifted: omits all 15 engineering+premium tools, misfiles `entity_delete_many`. Agents inspecting capabilities can't see the CLAUDE.md-mandated tools. | server.py:2070-2106 | — |
|
||||
| R16 | **MED** | COM `transaction_active` left stale-True if commit/rollback `_run` raises; `rollback` issues `EndUndoMark`+`UNDO B` SendCommand without the CMDACTIVE guard used elsewhere → can interleave mid-command. | com_backend.py:1402,1411-1421 | — |
|
||||
| R17 | **MED** | `_safe_send_command` busy-polls CMDACTIVE with `time.sleep(0.05)` on the single STA thread → monopolizes the executor up to deadline; `deadline_s`(8s) decoupled from `_run` timeout(60s). | com_backend.py:1521-1536 | — |
|
||||
| R18 | **MED** | `drawing_new` swallows bootstrap failures into `result['bootstrap']={'error':...}` and reports success → un-bootstrapped drawing looks normal though CLAUDE.md treats the scaffold as a precondition for gear_*/titleblock_*. | server.py:280-294 | — |
|
||||
| R19 | **MED** | `dimension_aligned`/`dimension_angular` permanently `xfail(strict=False)` — advertised tools shipped broken; an accidental fix wouldn't alert. | tests/test_dimensions.py:16-27 | — |
|
||||
| R20 | LOW | `_registered_tool_count` reaches into FastMCP privates (`mcp._local_provider._components`, `'tool:'` prefix); a minor upgrade → tool_count=-1 surfaced to clients. | server.py:231-234 | — |
|
||||
| R21 | LOW | Involute flank prepends a straight radial root→base segment (no trochoid fillet) → geometrically wrong dedendum for `root_r<base_r`; root-arc gap when `root_r>=base_r`; degenerate arc for teeth≤2. | engineering/gear.py:40-50,163-190 | — |
|
||||
| R22 | LOW | `drawing_undo`/`transaction_rollback` share one `_undo_stack` with no transaction-id/depth separation; reload doesn't reset `_doc_path`/`_current_layer`. | ezdxf_backend.py:470-484,1462-1476 | — |
|
||||
| R23 | LOW | COM `system_run_lisp` always reports `result='nil'` (SendCommand returns None); no CMDACTIVE guard → a prompting LISP form deadlocks next call. | com_backend.py:1487-1492 | — |
|
||||
| R24 | LOW | `_apply_attrs` sets `entity.dxf.linetype` without `_ensure_linetype_loaded` (unlike set_properties/layer_modify) → `linetype='HIDDEN'` on create may render Continuous or raise on save. | ezdxf_backend.py:491-501 | — |
|
||||
| R25 | LOW | `block_find_references` calls `entity_list(type_filter='INSERT')` with no limit (bypasses `max_list_limit`) → unbounded INSERTs into memory; assumes every backend populates `properties['block_name']`. | server.py:1362-1370 | — |
|
||||
| R26 | LOW | View zoom ops unconditionally registered but no-op on ezdxf → success response implies the view changed when it didn't; `view_zoom_and_screenshot` may not frame the requested region on ezdxf. | server.py:1814-1831,1870 | — |
|
||||
| R27 | LOW | `validator.py` title check substring-matches 'SPUR'/'HELICAL SPUR' on TEXT layer → false-positive blocks `drawing_finalize` if any note contains 'SPUR'. | engineering/validator.py:166-181 | — |
|
||||
| R28 | LOW | COM `_find_autocad_hwnd` grabs first visible window whose title contains 'AutoCAD' → may capture palette/splash/About dialog; not tied to `AcadApplication.HWND`. | com_backend.py:283-300 | — |
|
||||
| R29 | LOW | Broad `except Exception: pass` swallows COM faults (disconnected/busy) at debug level in ~20 sites → bypasses the HRESULT reconnect logic in `_run`. | com_backend.py:199,244,868,901,1216,… | — |
|
||||
|
||||
---
|
||||
|
||||
## 5. Doc-vs-Code Drift Table
|
||||
|
||||
| Claim (source) | Reality | Verdict |
|
||||
|---|---|---|
|
||||
| "87 tools" (CLAUDE.md, README:6) | **109** `@mcp.tool` (verified) | STALE (soft — both add "count is dynamic" caveat) |
|
||||
| "104 tests" (README:83) | **184** test functions across 14 files (verified) | STALE (~80 under) |
|
||||
| Section 1 "(11 tools)" (CLAUDE.md) | 12 (`drawing_redo` server.py:426) | WRONG |
|
||||
| Section 9 "(5 tools)" (CLAUDE.md) | 4 view tools registered | WRONG |
|
||||
| "12-section taxonomy" (CLAUDE.md) | 1–12 + 8b/8c/8d + 13 = 6 extra groups | INCOMPLETE |
|
||||
| Premium workflow "non-negotiable for production" (CLAUDE.md) | 8/8 premium tools dead on COM (default Windows backend) | OVERCLAIM |
|
||||
| "MIT — see [LICENSE](LICENSE)" (README:409) | No LICENSE file (verified absent) | **DEAD LINK / legal drift** |
|
||||
| `benchmarks/` referenced (README:39,302,367) | Absent (verified) | DEAD LINK (forward-looking) |
|
||||
| ".github/workflows/ci.yml" + "GitHub Actions CI" (README:84,358) | git status `D`; absent on disk (verified) | **CI REMOVED while docs claim it** |
|
||||
| "Premium discipline lives in `.claude/skills/autocad-mcp-premium/`" (CLAUDE.md) | Entire tree git-deleted (28 deletions) + all `.cursor/` deleted | DANGLING POINTER |
|
||||
| README tests/ tree (README:347-355) | Omits 6 of 14 test files + engineering/ package | STALE |
|
||||
| `system_about` tool_groups | Omits 15 engineering+premium tools | DRIFT |
|
||||
| `BlockInfo.description` field (base.py:71) | Never populated by either backend | DEAD FIELD |
|
||||
| critique.py:53 "normalises to mm" comment | Stores raw DXF lineweight (ezdxf:189) | FALSE COMMENT |
|
||||
| `engineering/__init__.py:37-39` "Agent B … in flight"; com_backend "Task #7/#8/#9" | Multi-agent build seams shipped unreconciled | TODO-IN-PROD |
|
||||
| Dockerfile, .env.example (README) | Present (verified) | OK |
|
||||
|
||||
---
|
||||
|
||||
## 6. Test Coverage Gaps (ranked)
|
||||
|
||||
1. **COM backend: 0% coverage** (com_backend.py entire file) — transactions (UndoMark vs ezdxf snapshot are *different* semantics), SendCommand corner ops, CMDACTIVE guard, timeout/HRESULT recovery, all 8 premium stubs. Highest-risk untested surface; it's the production path. (R6)
|
||||
2. **Server→sanitizer integration** — no test proves tools invoke `sanitize_command/lisp`/`validate_path`. (R7)
|
||||
3. **Sanitizer adversarial cases** — LISP aliasing, command word-boundary evasion (`.ERASE`), UNC/`~`/symlink path escapes — the comment claims defenses with no tests. (R8)
|
||||
4. **Critique detection paths** — `iso128`, `layer_color`, `dim_overlap` only exercised on a *clean* drawing (assert they *don't* fire); their actual detection logic and the unknown-focus branch (critique.py:241) are never proven. (R3, R4)
|
||||
5. **`dimension_auto` ordinate** (ezdxf:1988) untested; chain/baseline covered.
|
||||
6. **`dimension_aligned`/`angular`** permanently xfail — shipped broken. (R19)
|
||||
7. **Uncovered backend methods (both backends):** `system_get/set_variable`, `system_status`, `drawing_close`, `drawing_export_pdf`, `entity_create_block_ref`, `block_set_attributes`, `linetype_list/load` (load-bearing for bootstrap), `view_zoom_*`, `view_screenshot`.
|
||||
8. **`engineering/section.py`** (dead code) — 0 coverage.
|
||||
9. **Transaction nesting/error semantics** — only single begin→commit/rollback tested; nested, rollback-without-begin, layer/block restoration untested.
|
||||
10. **Infrastructure:** CI workflow deleted (no PR gate); `pytest-cov` declared but no `--cov`/`fail-under`; several "template/validation" tests assert nothing about the named feature (vacuous `if start:` guards, test_batch_template.py:30-34).
|
||||
|
||||
---
|
||||
|
||||
## 7. Research Synthesis
|
||||
|
||||
### 7a. Academic techniques (2025–2026) that validate or upgrade this design
|
||||
|
||||
- **GeoGramBench (ICLR 2026, arxiv.org/abs/2505.17653):** frontier LLMs score **<50%** at high-abstraction Program-to-Geometry. → Empirically justifies the repo's no-coordinate-guessing rule and `point_from_snap`/`entity_select_smart`. **Action:** expand the deterministic geometry surface (`point_intersection`, `point_offset`, `point_along`) so the LLM *never* computes coordinates.
|
||||
- **Self-Improving CAD w/ FEA feedback (arxiv.org/html/2605.17448v2):** deterministic controller owns validation, LLM owns design; **+13.4pp** per feedback round; frontier models get **zero** first-attempt strict passes — *iteration beats more tokens*. → This is the repo's architecture, but `drawing_critique` is one-shot. **Action:** make it a routed repair loop; `Issue` (plan_spec.py:64-81) already carries severity/handles/detail — add per-finding repair hints.
|
||||
- **CADSmith (arxiv.org/abs/2603.26512):** 5-agent Planner/Coder/Executor/Validator/**Refiner**, dual loop, exact-kernel measurements + **independent VLM judge** → Chamfer 28.37mm→0.74mm (**38×**), IoU 0.81→0.96; *removing the rendered image alone made hard cases 35× worse.* → The repo renders a screenshot at finalize (server.py:2301-2309) but only checks bytes exist (validator step 7). **Highest-leverage single upgrade: feed that render to a VLM judge + add an automated Refiner.**
|
||||
- **CADFusion (ICML 2025, arxiv.org/abs/2501.19054)** & **Seek-CAD (arxiv.org/abs/2505.17702):** render is a first-class signal; step-wise visual feedback catches errors mid-build. → Add a `visual` critique focus; run critique per construction step using the existing transaction model.
|
||||
- **ProCAD "Clarify Before You Draw" (arxiv.org/html/2602.03045v1):** one-round clarification → invalidity 14.6%→0.9%, Chamfer ~80% lower. → `drawing_plan` records intent passively; add a pre-plan ambiguity/conflict detector that returns targeted questions before any `entity_create_*`.
|
||||
- **FutureCAD (arxiv.org/abs/2603.11831):** NL → exact B-Rep primitive grounding fixes the fillet/chamfer "which primitive" problem. → Directly targets the repo's hardest API: trim/extend/fillet/chamfer need exact handles **plus** keep_x/keep_y. NL grounding ("fillet the top-left corner") removes the burden GeoGramBench shows LLMs fail at.
|
||||
- **Text-to-CadQuery (2505.06507), CAD-Coder (2505.14646), EvoCAD (2510.11631):** field converged on Python/CadQuery macros; open local VLMs read CAD images reliably; candidate-generate + critique-as-fitness raises first-try quality. → Add more parametric meta-tools (the gear/keyway pattern) and/or a sandboxed parametric-script tool; an open VLM (CAD-Coder, **Florence-2 for GD&T**, arxiv.org/abs/2411.03707) keeps the visual tier off frontier APIs.
|
||||
- **MUSE (arxiv.org/html/2605.28579) + CadBench (2605.10873):** grade manufacturability/functionality, not just shape; standard metrics = volumetric/surface IoU, Chamfer, **Invalidity Ratio**. → Replace `validator.py`'s regex/title heuristics with a staged code→geometric→rubric-judge protocol; emit a single scalar "drawing score" for regression tests (the repo has *no* quantitative quality metric).
|
||||
|
||||
### 7b. Competitive landscape
|
||||
|
||||
| Product | 3D | Render-critique loop | Validation gate | HTTP auth | Code escape hatch |
|
||||
|---|---|---|---|---|---|
|
||||
| **AutoCAD MCP Pro** | ❌ 2D only | ⚠️ finalize-only, symbolic | ✅ **unique** (critique+validator+ISO) | ❌ none | ⚠️ COM-only run_command/lisp |
|
||||
| build123d-mcp/agentcad (github.com/pzfreo/build123d-mcp) | ✅ | ✅ in-loop | ❌ | — | ✅ |
|
||||
| FreeCAD Robust MCP (150+ tools) | ✅ + FEM | ⚠️ | ❌ | — | ✅ |
|
||||
| Blender MCP (~22k★) | ✅ | ✅ | ❌ | — | ✅ Python exec |
|
||||
| RhinoMCP | ✅ | ✅ viewport | ❌ | — | ✅ |
|
||||
| Fusion360/Onshape MCP | ✅ parametric | — | ❌ | ✅ OAuth/keys | — |
|
||||
|
||||
Snyk's 9-CAD-MCP roundup (snyk.io/articles/9-mcp-servers-for-computer-aided-drafting-cad-with-ai/): **vision only in FreeCAD/Blender/Rhino; design validation in none; HTTP auth only in Onshape/Fusion.** → **Our moat is the design-rule validation gate; the field beat us on 3D, in-loop render-critique, and auth.** Autodesk's own in-AutoCAD assistant demo (aecmag.com/features/autodesk-shows-its-ai-hand/) flagship = **automated compliance checking** — exactly our niche, so we must make standards compliance the sharpest, most-cited capability. Adam/Zoo (techcrunch.com/2025/10/31/...) set the UX bar: **parametric editability + post-gen dimension sliders + entity-level conversational editing**.
|
||||
|
||||
### 7c. Under-used COM / ezdxf API + ISO standards
|
||||
|
||||
**COM (ActiveX) gaps (every create call hits ModelSpace, com_backend.py:125-127):**
|
||||
- **PaperSpace/Layouts + Plot config** — `drawing_export_pdf` (com_backend.py:528-534) calls bare `Plot.PlotToFile(path,'DWG To PDF.pc3')` with no page setup → ignores the A3/scale `drawing_plan` promised. Add `layout_*`/`viewport_*` (AddPViewport, StandardScale, CanonicalMediaName). (help.autodesk.com ActiveX ref)
|
||||
- **AddTable** (BOM/revision tables), **AddMLeader** (ISO leader notes — CLAUDE.md *reserves* "leader notes" but no tool creates them), **Fields** (auto-updating title block), **GetDynamicBlockProperties** (parametric symbol libraries — `block_insert` only does attribute text, com:1195-1251), **DimStyles.Add+CopyFrom** (no ISO-129 dimstyle is ever set → undercuts the "Dimensions are ISO 129" rule), **named UCS** (per-view frames for `view_count`), **AttachExternalReference** (shared borders), **gradient/island hatch**.
|
||||
- **3D solids/regions/booleans** — `system_status` advertises `all_entity_types` (com:1443) which is **false**; either implement `solid_*` or correct the claim.
|
||||
- **Push selection into AutoCAD:** `entity_list` (com:967) and `analysis_select_by_type` (com:1352) loop ModelSpace.Item(i) in Python (O(n) COM round-trips — the #1 perf killer); `analysis_select_by_layer` (com:1339) already shows the right `SelectionSet.SelectAll` group-code-filter pattern to copy.
|
||||
|
||||
**ezdxf gaps (Grep confirms zero use of `set_gradient/set_solid_fill/paperspace/add_viewport/odafc/export_dwg/groupby/dimstyles/MTextEditor/add_ordinate/qsave`):**
|
||||
- **`addons.odafc.export_dwg`** = real DWG (R12–R2018) → fixes R11/R10 (drawing_save_as fake DWG). (ezdxf.mozman.at/docs/addons/odafc.html)
|
||||
- **drawing-addon `Configuration`** (lineweight_policy ABSOLUTE, background_policy, SVGBackend/PyMuPdfBackend) → true ISO lineweights + vector output, replacing raw matplotlib (ezdxf:357-379,1400-1424).
|
||||
- **`query()` mini-language + `groupby()`** → replace hand-rolled loops in analysis_stats/select/entity_select_smart (which pulls 5000 entities then filters in memory, ezdxf:2008-2050).
|
||||
- **`recover.readfile` + `Auditor.fix`** → real repair; `drawing_audit` (ezdxf:448) reports but never fixes; `drawing_open` (ezdxf:309) has no recover fallback.
|
||||
- **`MTextEditor`** (stacked tolerance fractions), **Hatch gradients/islands/edge-paths**, **DimStyle overrides** (no ISO dimstyle today, ezdxf:668-774), **path-module offset** (generalizes LINE/CIRCLE-only offset + LINE-only corner ops).
|
||||
|
||||
**ISO standards conformance:**
|
||||
- ✅ **ISO 128-2** lineweight set encoded (plan_spec.py:85, verified) — but **nulled by R2** and group-vs-element-class not checked.
|
||||
- ⚠️ **ISO 7200** title block — A3-only hardcoded (titleblock.py:16-17), informal labels ("DWG NO" vs ISO field names), len×0.3 centering heuristic (titleblock.py:171), missing "Document type"/"Date of issue".
|
||||
- ❌ **ISO 129-1 dimensioning** — biggest functional gap: `dimension_*` (base.py:226-257) have **no** param for ±/limit/fit (H7/g6) tolerances or text override → toleranced production dims impossible. (iso.org/obp/ui/#iso:std:iso:129)
|
||||
- ❌ **ISO 1101 / ASME Y14.5 GD&T** — entirely absent (grep-confirmed): no feature control frame, datum, or ISO 2768 general-tolerance note. (gdandtbasics.com/iso-vs-asme-standards/)
|
||||
- ⚠️ **ISO 13567** — static mech-only starter list (layers.py:51-62), no field-grammar parser/validator.
|
||||
|
||||
---
|
||||
|
||||
## 8. "What Would Make This the BEST AutoCAD MCP" — Prioritized Thesis
|
||||
|
||||
The evidence converges: **the repo is architecturally ahead on dual-backend + standards-validation but behind on the closed-loop, the production-output plumbing, and the very workflow it markets.** Priorities, evidence-backed:
|
||||
|
||||
**P0 — Make the marketed product actually work on its default platform.**
|
||||
1. **Implement the 8 premium meta-tools on COM** (R1, com_backend.py:1637-1660). Today the CLAUDE.md-mandatory workflow 500s on every Windows install. Nothing else matters until this ships.
|
||||
2. **Fix the lineweight `int()` truncation** (R2, ezdxf_backend.py:1034 → multiply mm by 100 / map to ACAD enum). This single bug nullifies the ISO-128 system *and* makes the iso128 critique a permanent no-op (R3). The standards-validation gate is the **competitive moat** (Snyk roundup; Autodesk's own flagship demo) — it must be real.
|
||||
3. **Add CI + a coverage gate + the first COM smoke tests** (R6, ci.yml deleted; pytest-cov unconfigured). Restore the LICENSE file (legal drift).
|
||||
|
||||
**P1 — Own the moat by closing the loop (the #1 accuracy lever in every 2025–26 paper).**
|
||||
4. **Turn `drawing_critique` into a routed iterative Refiner loop** with per-`Issue` repair hints and a bounded budget (CADSmith Refiner; FEA-agent +13.4pp/round; Physics-in-the-Loop 3.3–6.0 iters converge). The `Issue` dataclass already carries handles+detail.
|
||||
5. **Promote the finalize screenshot to a first-class `visual` critique focus fed to a VLM judge** (CADSmith: dropping the render → 35× worse; CADFusion). Use an **open local VLM** (CAD-Coder / Florence-2 for GD&T) behind a `[vision]` extra so the headless path is unaffected. Add a **dimension round-trip check** (VLM-read dims vs created `dimension_*` entities) to catch what proximity-only `dim_overlap` (R4) cannot.
|
||||
6. **Add a ProCAD-style pre-plan clarification pass to `drawing_plan`** (invalidity 14.6%→0.9%): detect missing/conflicting dims, material, tolerance, view_count-vs-intent before any geometry.
|
||||
7. **Emit a scalar "drawing score" / Invalidity-Ratio** (MUSE/CadBench) so the premium workflow is regression-testable — the repo has no quantitative accuracy metric.
|
||||
|
||||
**P2 — Reach production-drawing parity (what un-toleranced ISO 128 can't deliver).**
|
||||
8. **ISO 129 tolerances + ISO 1101/Y14.5 GD&T**: add ±/limit/fit params to `dimension_*` and a `gd_frame(symbol, tol, datums, modifiers)` + datum/ISO-2768-note primitive. This is the categorical gap blocking real part drawings; no surveyed text-to-CAD product covers 2D GD&T — a differentiator.
|
||||
9. **Real plotted sheets**: COM Layout/PaperSpace/Plot-config so `drawing_plan(sheet_size, scale)` and `titleblock_apply_iso_a3` produce a correct A-series plot; on ezdxf wire `odafc.export_dwg` (kills the fake-DWG R11) and the drawing-addon `Configuration` for true-lineweight SVG/PDF.
|
||||
10. **An ISO-129 `DimStyle`** created once and applied to all dims (COM `DimStyles.Add+CopyFrom`; ezdxf dimstyle overrides) so "Dimensions are ISO 129" becomes true, and **MLEADER** for the leader notes the docs already reserve.
|
||||
|
||||
**P3 — Catch the field on breadth (table-stakes elsewhere, absent here).**
|
||||
11. **A 3D solids tier** (extrude/revolve from 2D profiles + booleans on COM; STEP/STL on ezdxf) — *or* correct the false `all_entity_types` claim (com:1443). Every competitor has 3D.
|
||||
12. **Parametric editability + entity-level conversational editing** (Adam/Zoo): capture driving dims in PlanSpec, expose a re-drive tool; "select this circle → make it the 20mm bore" via existing handle model + `entity_select_smart`/`point_from_snap`.
|
||||
13. **Authenticated remote HTTP** (API-key middleware) given `dangerous_commands_enabled` exists (server.py:111); a **standard-parts/symbol library** (`library_insert` over `block_insert`, dynamic-block props); **NL→B-Rep grounding** (FutureCAD) over the brittle trim/fillet/chamfer keep-point API.
|
||||
|
||||
**P4 — Hygiene that compounds with the above.** Generalize LINE-only corner ops + LINE/CIRCLE-only offset via the ezdxf path module; replace hand-rolled selection with `query()`/`groupby()` (ezdxf) and `SelectionSet.SelectAll` filters (COM); fix `entity_array_polar` 360° dup (R9), `dimension_angular` tx/ty (R12), the COM thread-leak `CoUninitialize` (R5); delete dead `section.py`/`generate_tooth_profile`; reconcile `system_about` catalog (R15) and the stale README/CLAUDE.md drift (§5).
|
||||
|
||||
**Files central to every priority:** `backends/com_backend.py:1637-1660` (P0.1), `backends/ezdxf_backend.py:1034` + `engineering/layers.py:14-26` (P0.2), `engineering/critique.py:37-64,194-197` (P0.2/P1.5), `engineering/plan_spec.py:64-87` (P1.4/P1.6/P2.8), `backends/base.py:226-257` (P2.8), `server.py:528-534/2294-2309` (P2.9).
|
||||
@@ -0,0 +1,309 @@
|
||||
<!-- Generated by the DIAGNOSTICS workflow (wf_c3e5770d-2cf), 2026-06-18.
|
||||
Verified: 31 | Confirmed: 28 | Refuted/fixed: 2 | New from sweep: 17.
|
||||
NOTE: R7/R8 (sanitizer wiring + LISP-aliasing/path-traversal defensive tests) were
|
||||
NOT auto-verified — the security subagents hit a provider cyber-safety filter.
|
||||
They are tracked for manual defensive review during the hardening work item. -->
|
||||
|
||||
# DIAGNOSTICS Report — AutoCAD MCP Pro
|
||||
|
||||
Synthesized from the verified recon risk register + fresh-sweep findings. REFUTED / ALREADY-FIXED items are excluded from the ranked body and listed in the appendix. Duplicates between the two sources are merged. Line numbers reflect **current** code (verified against the working tree).
|
||||
|
||||
---
|
||||
|
||||
## CRITICAL
|
||||
_None._
|
||||
|
||||
---
|
||||
|
||||
## HIGH
|
||||
|
||||
### N1 — COM `entity_create_hatch` builds an ASSOCIATIVE hatch then deletes its boundary polyline
|
||||
- **Severity:** HIGH
|
||||
- **Location:** `backends/com_backend.py:659-673`
|
||||
- **Description:** `mspace.AddHatch(0, pattern, True)` requests an *associative* hatch linked to the appended boundary loop, but the code then calls `bnd_pline.Delete()` (`com_backend.py:673`) on that very polyline. In live AutoCAD this either errors on `Delete`, drops the boundary, or leaves a hatch whose boundary tracking breaks on the next regen/edit.
|
||||
- **Fix:** Change the 3rd arg to non-associative — `AddHatch(0, pattern, False)` — so the computed fill survives deleting the throwaway boundary. If associativity is truly wanted, keep the boundary polyline instead of deleting it.
|
||||
|
||||
### N2 — `drawing_save_as` default format diverges (ezdxf `"dxf"` vs COM/ABC `"dwg"`); `drawing_finalize` relies on the default
|
||||
- **Severity:** HIGH (merges register **R10**/**R11** with the fresh-sweep duplicate)
|
||||
- **Location:** ABC default `backends/base.py:158` (`"dwg"`); ezdxf `backends/ezdxf_backend.py:346` (`"dxf"`); COM `backends/com_backend.py:516` (`"dwg"`); consumed with no fmt at `server.py:2299` (`drawing_finalize` → `backend.drawing_save_as(str(validated))`).
|
||||
- **Description:** `drawing_finalize` passes no `fmt`, so each backend's default decides the output. Two distinct real defects layer here:
|
||||
1. **ezdxf silently mislabels DWG (R11):** `drawing_save_as` *ignores* `fmt` entirely — `doc.saveas(path)` derives format from the extension — yet still returns `{"format": fmt}`. For `fmt="dwg"`/`"dwt"` it writes DXF tag-stream bytes (ezdxf has no DWG/DWT writer) while echoing `format:"dwg"`. (`ezdxf_backend.py:346-353`)
|
||||
2. **COM writes DWG content under a `.dxf` name (R10):** COM's `SaveAs(path, fmt_map[fmt])` honors `fmt`, *not* the extension, so a `.dxf` save_path under COM yields DWG bytes in a `.dxf`-named file.
|
||||
- Net: identical `drawing_finalize(save_path=...)` produces different on-disk formats per backend, decoupled from the extension the user supplied.
|
||||
- **Fix:** In `drawing_finalize`, derive `fmt` from the `save_path` extension and pass it explicitly. Unify the ABC + both impl signatures to one default. In ezdxf, reject `fmt not in {dxf, dwt}` (raise `ToolError` for `dwg`) instead of writing DXF and echoing `format=fmt`.
|
||||
|
||||
---
|
||||
|
||||
## MED
|
||||
|
||||
### R4 — `dim_overlap` critique focus is a guaranteed silent no-op (no backend populates the keys it reads)
|
||||
- **Severity:** MED (claimed HIGH; downgraded — silently returns `[]`, no incorrect output)
|
||||
- **Location:** reads `engineering/critique.py:199-201`; extractors `backends/ezdxf_backend.py:167-169` and `backends/com_backend.py:204-244` (verified at `critique.py:199-201`).
|
||||
- **Description:** `_check_dim_overlap` reads `text_position`/`defpoint`/`insertion_point`, but the ezdxf DIM branch sets only `props["dim_type"]` and COM dims get only a generic `bounding_box`. None of the three keys is produced anywhere, so `ip` is always falsy, `dim_points` stays empty, and the focus always returns `[]`.
|
||||
- **Fix:** Populate a dim reference point (text midpoint / defpoint) in both `_entity_info` extractors for DIM types, OR fall back to `properties["bounding_box"]` center in `_check_dim_overlap`.
|
||||
|
||||
### R9 — ezdxf `entity_array_polar` overlaps the original on full-circle fill and diverges from COM
|
||||
- **Severity:** MED
|
||||
- **Location:** `backends/ezdxf_backend.py:947-948`
|
||||
- **Description:** `step = radians(fill_angle) / max(count-1, 1)` with loop `range(1, count)` places the last copy at `step*(count-1) = 360°` for a full ring — coincident with the original at 0°. Also diverges from COM (`com_backend.py:921` uses AutoCAD `ArrayPolar`, spacing `360/count`): `count=4, fill=360` → COM 90/180/270 vs ezdxf 120/240/360. `count-1` is only correct for partial-arc fills.
|
||||
- **Fix:** `divisor = count if abs(fill_angle % 360) < eps else max(count-1, 1)`.
|
||||
|
||||
### R12 — ezdxf `dimension_angular` ignores `tx/ty` and hardcodes `distance=10.0`
|
||||
- **Severity:** MED
|
||||
- **Location:** `backends/ezdxf_backend.py:706-723` (hardcode at `:715`)
|
||||
- **Description:** Accepts `tx/ty` (advertised as "text position X/Y" at `server.py:721-722`) but calls `add_angular_dim_2l(..., distance=10.0)`, ignoring them. COM honors `tx/ty` via `AddDimAngular` (`com_backend.py:770-784`), so identical inputs place the arc differently per backend; the fixed 10.0 mislocates the arc for very large/small parts.
|
||||
- **Fix:** Pass `location=(tx,ty)` to `add_angular_dim_2l`, or compute `distance=hypot(tx-vx, ty-vy)`.
|
||||
|
||||
### R13 / N2-offset — `entity_offset` ignores `side_x/side_y` in BOTH backends; side semantics diverge; ezdxf supports only LINE/CIRCLE
|
||||
- **Severity:** MED (merges register **R13** with the two fresh-sweep offset findings)
|
||||
- **Location:** ezdxf `backends/ezdxf_backend.py:869-909`; COM `backends/com_backend.py:861-875`; ABC sig `backends/base.py:315-319`; tool `server.py:859-868`.
|
||||
- **Description:** Both backends accept `side_x/side_y` and never read them (dead params; the tool advertises them as "a point on the offset side"). ezdxf hard-fixes the LINE normal `Vec2(-dir.y, dir.x)*d` (`:882`) and CIRCLE `radius+d` (`:896`); every other type raises `RuntimeError("Offset not supported …")` (`:908`) — ARC/LWPOLYLINE/POLYLINE/ELLIPSE/SPLINE unsupported, contradicting the tool's "line, circle, or polyline" claim. COM delegates to `ent.Offset(distance)`, whose sign/side convention is not guaranteed to match ezdxf's fixed left-normal — same `(handle, distance)` can offset to opposite sides per backend.
|
||||
- **Fix:** Honor `side_x/side_y` in both backends (pick the copy whose ref point is nearest the side point, or sign `distance` from the cross product against the offset direction). In ezdxf, support ARC/LWPOLYLINE/POLYLINE/ELLIPSE or drop the documented polyline/side claims. Reconcile so positive distance means the same side in both engines.
|
||||
|
||||
### R15 — `system_about` `tool_groups` static dict omits ~20 registered tools and misfiles one
|
||||
- **Severity:** MED
|
||||
- **Location:** `server.py:2070-2106`
|
||||
- **Description:** The hand-maintained `tool_groups` dict omits `drawing_close`, `entity_trim/extend/fillet/chamfer`, all 7 engineering tools, and all 8 premium tools (~15+ shown), and lists `entity_delete_many` under `entity_creation` though it's tagged `{entity, modify}` with `destructiveHint=True`. Live `total_tools` stays correct; only the per-group breakdown drifts.
|
||||
- **Fix:** Derive `tool_groups` dynamically from registered tools' tags, or at minimum add the engineering/premium groups, move `entity_delete_many`, and add the 5 missing modify/drawing tools.
|
||||
|
||||
### R18 — `drawing_new` bootstrap failures return as success
|
||||
- **Severity:** MED
|
||||
- **Location:** `server.py:282-293`; per-item swallow at `engineering/layers.py:90-91, 128-129`
|
||||
- **Description:** Bootstrap is wrapped in `try/except` that stores `{"error": …}` and still `return result` representing a created drawing; per-item failures are caught internally as `"failed: {exc}"` strings. A fully- or partially-failed bootstrap looks like success, so geometry can land on missing engineering layers — a documented precondition violation.
|
||||
- **Fix:** Surface `bootstrap_ok=False` / `status="degraded"` and/or raise `ToolError` when `bootstrap=True` and any linetype/layer reports `failed` or the catch-all fires.
|
||||
|
||||
### R19 — ezdxf `dimension_aligned` / `dimension_angular` ship broken (xfail strict=False masks fix/regression)
|
||||
- **Severity:** MED
|
||||
- **Location:** markers `tests/test_dimensions.py:16, 23`; root cause `backends/ezdxf_backend.py:696` (`dist=`) and `:711-715` (`center=/p1=/p2=/distance=`)
|
||||
- **Description:** ezdxf 1.4.3 `add_aligned_dim()` is `(p1, p2, distance, …)` but code passes `dist=`; `add_angular_dim_2l()` is `(base, line1, line2, …)` but code passes `center=/p1=/p2=/distance=` → `TypeError`. Both are advertised MCP tools (`server.py:695, :714`) and recommended in prompt text (`:2639`). `xfail(strict=False)` means an accidental fix XPASSes silently.
|
||||
- **Fix:** `dist→distance`; `base=(vx,vy)`, `line1=((vx,vy),(x1,y1))`, `line2=((vx,vy),(x2,y2))`. Change markers to `strict=True` (or remove) so the suite alerts.
|
||||
|
||||
### R22 — ezdxf transaction snapshots share the single `_undo_stack` with undo history; reload leaves `_current_layer` stale
|
||||
- **Severity:** MED
|
||||
- **Location:** `backends/ezdxf_backend.py:213` (single stack), `drawing_undo` `471-485`, `transaction_begin/commit/rollback` `1429-1477`, `system_status` `1489`
|
||||
- **Description:** One `_undo_stack` is pushed/popped by both undo and transactions, with no txn id/marker/depth isolation; `system_status` reports `len(_undo_stack)` *as* `transaction_depth`. A `drawing_undo` silently consumes an open transaction's snapshot. On reload, undo/rollback re-read the doc but never reset `_current_layer`, so `is_current` reporting goes stale vs the snapshot's `$CLAYER`.
|
||||
- **Fix:** Use a dedicated transaction list (or begin-markers/ids) distinct from undo history; on reload re-read `$CLAYER` into `self._current_layer`.
|
||||
|
||||
### R5 — COM thread/apartment leak on `SendCommand` timeout (no `CoUninitialize`)
|
||||
- **Severity:** MED
|
||||
- **Location:** `backends/com_backend.py:416-434` (timeout path), `69-72` (`_com_init`), `387-391` (disconnect)
|
||||
- **Description:** `_com_init` calls `CoInitialize()` as the executor initializer; `CoUninitialize` appears nowhere in source. On `TimeoutError`, `stuck.shutdown(wait=False)` cannot cancel a worker still blocked in `SendCommand` (the inline comment admits "Abandon it"), and a fresh executor is built — each timeout permanently leaks one `CoInitialize`'d thread + the COM apartment it pinned. `disconnect()` has the same pattern.
|
||||
- **Fix:** Bound the leak (track abandoned executors, avoid unbounded re-creation); add per-thread `CoUninitialize` via worker-side `try/finally`/thread-local teardown so apartments release when a timed-out thread eventually unblocks.
|
||||
|
||||
### R6 — COM backend (1651 lines, 85 async methods) has zero test coverage
|
||||
- **Severity:** MED
|
||||
- **Location:** `tests/conftest.py:5` (only `EzdxfBackend` imported); untested logic incl. CMDACTIVE guard `com_backend.py:1474`, transactions `1397-1419`
|
||||
- **Description:** No test references `ComBackend`/`win32`/`SendCommand`/`CMDACTIVE`. The `backend` fixture instantiates only ezdxf. SendCommand-driven commands, the CMDACTIVE busy-guard + poll loop, and transaction begin/commit/rollback are unexercised, so COM logic can regress silently on non-Windows CI.
|
||||
- **Fix:** Add `tests/test_com_backend.py` unit-testing the mockable logic (CMDACTIVE guard, `normalize_lineweight` usage, `_safe_send_command` polling, value coercion) with a mocked COM `app`/`doc`, gated by `@pytest.mark.skipif` when `win32com` is unavailable.
|
||||
|
||||
### N3 — `entity_select_smart` `length_range` always rejects ARCs (no backend populates ARC `"length"`)
|
||||
- **Severity:** MED
|
||||
- **Location:** `backends/base.py:897-904`
|
||||
- **Description:** The gate fires for `("LINE","ARC")` and reads `e.properties.get("length")`, but the ARC extractor branches (`ezdxf_backend.py:135-139`, `com_backend.py:217-222`) set only center/radius/angles — never `length`. So every ARC has `length is None → return False`, silently excluding all arcs despite the advertised ARC support.
|
||||
- **Fix:** Populate `props["length"]` for ARC (= `radius * |Δangle|` in radians, with wraparound) in both extractors, OR compute arc length on the fly in `entity_select_smart`, OR drop "ARC" from the gate and docstring.
|
||||
|
||||
### N4 — `dimension_auto` hardcodes `layer="DIM"`, which does not exist in the `iso13567` layer set
|
||||
- **Severity:** MED
|
||||
- **Location:** `backends/base.py:822, 836, 845, 852`
|
||||
- **Description:** Every `dimension_linear` inside `dimension_auto` passes `layer="DIM"`, but the iso13567 set's dim layer is `"M-DIMEN-T-N"` (`engineering/layers.py:57`). After `drawing_apply_iso_layers("iso13567")`, dims land on an auto-created `"DIM"` (default color 7 / lw -3), bypassing the ISO layer. `_check_layer_color` won't flag it because `_EXPECTED_LAYER_COLOR` is built only from `ENGINEERING_LAYERS`. (The hardcoded `"CONSTRUCTION"` in `construction_xline/clear`, `base.py:743/:754`, has the same iso13567 mismatch — its construction layer is `"M-CONST-E-N"`.)
|
||||
- **Fix:** Resolve the dimension (and construction) layer from the active layer set via a registry lookup keyed on `PlanSpec.layer_set_id` (mech→DIM, pid→DIM, iso13567→M-DIMEN-T-N) instead of literals.
|
||||
|
||||
### N5 — `EntityInfo.properties` shape diverges by backend (`bounding_box`, TEXT `rotation`, MTEXT `char_height`)
|
||||
- **Severity:** MED (merges three fresh-sweep property-parity findings)
|
||||
- **Location:** `bounding_box`: `com_backend.py:192-199` (every entity) vs `ezdxf_backend.py:122` (never); TEXT/MTEXT: `ezdxf_backend.py:144-152` vs `com_backend.py:229-237`.
|
||||
- **Description:** COM seeds `properties["bounding_box"]` for *every* entity; ezdxf never adds it. ezdxf TEXT sets `rotation` and MTEXT sets `char_height`; COM TEXT/MTEXT set neither (COM MTEXT sets no height at all). Any client reading these keys works on one backend and `KeyError`s on the other.
|
||||
- **Fix:** Pick a contract and enforce it both ways: add `bounding_box` to ezdxf via `ezdxf.bbox.extents` (or drop it from COM and expose extents only via `analysis_bounding_box`); add `rotation`/`char_height` to the COM TEXT/MTEXT branches (`rad2deg(entity.Rotation)`, `entity.Height`). Document the props contract.
|
||||
|
||||
### N6 — COM `system_run_lisp` always reports `result:"nil"` (SendCommand is void)
|
||||
- **Severity:** MED (duplicates register **R23**'s value-capture half; R23's no-CMDACTIVE-guard half tracked at LOW below)
|
||||
- **Location:** `backends/com_backend.py:1488-1493`
|
||||
- **Description:** `result = doc.SendCommand("(progn …)")` then `str(result) if result else "nil"`. AutoCAD COM `SendCommand` is void-returning → `result` is always `None` → the tool unconditionally returns `"nil"` regardless of the actual LISP value.
|
||||
- **Fix:** Drop the bogus `result` field, or capture out-of-band: `(setvar "USERS1" (vl-princ-to-string <expr>))` then `GetVariable("USERS1")` after completion.
|
||||
|
||||
---
|
||||
|
||||
## LOW
|
||||
|
||||
### R10/R11 residual — _covered under N2 (HIGH)._ No separate LOW entry; the format-default divergence is fully captured there.
|
||||
|
||||
### R16 — COM `transaction_commit/rollback` leave `_transaction_active` stale on error; rollback's `UNDO B` bypasses the CMDACTIVE guard
|
||||
- **Severity:** LOW
|
||||
- **Location:** `backends/com_backend.py:1406-1423` (commit `1406-1413`, rollback `1415-1423`; begin mirror at `1402-1403`)
|
||||
- **Description:** `_transaction_active` is set/cleared *after* `await self._run(_sync)` with no `try/finally`; if `_run` raises, the flag stays stale. Rollback's `_sync` issues raw `doc.SendCommand("_UNDO B\n")` (`:1419`) without the CMDACTIVE pre-check/poll that `system_run_command`/`_safe_send_command` use — the documented deadlock hazard.
|
||||
- **Fix:** Clear `_transaction_active` in a `finally` around `_run`; route rollback's `UNDO B` through `_safe_send_command`.
|
||||
|
||||
### R20 — `_registered_tool_count()` reaches through FastMCP private attrs; fails to `-1` and surfaces it
|
||||
- **Severity:** LOW
|
||||
- **Location:** `server.py:225-234` (private access at `:231-232`)
|
||||
- **Description:** `mcp._local_provider._components` + the undocumented `"tool:"` key prefix, with `except Exception: return -1`. `-1` is surfaced unguarded as `tool_count` (`server.py:1969/1976/1983`) and `total_tools` (`:2107`). A minor `fastmcp>=3.0` restructuring silently degrades the count to `-1`.
|
||||
- **Fix:** Use the public API (e.g. `len(await mcp.get_tools())`), making the helper async.
|
||||
|
||||
### R23 — COM `system_run_lisp` has no CMDACTIVE guard (deadlock on prompting LISP)
|
||||
- **Severity:** LOW (value-capture half merged into N6)
|
||||
- **Location:** `backends/com_backend.py:1488-1493`
|
||||
- **Description:** Unlike `system_run_command` (guards at `1473-1482`) and `_safe_send_command` (polls at `1521-1537`), `system_run_lisp` sends raw with no CMDACTIVE pre-check/poll. A prompting form (`(command "_LINE")`, `(getpoint)`) leaves CMDACTIVE non-zero and deadlocks the next COM call.
|
||||
- **Fix:** Route through `_safe_send_command` (or add the same pre-send guard + post-send poll).
|
||||
|
||||
### R24 — ezdxf `_apply_attrs` assigns linetype without `_ensure_linetype_loaded` → undefined names silently render Continuous
|
||||
- **Severity:** LOW
|
||||
- **Location:** `backends/ezdxf_backend.py:501-502`
|
||||
- **Description:** `_apply_attrs` does `entity.dxf.linetype = linetype` with no load check, unlike `entity_set_properties:981`, `layer_create:1030`, `layer_modify:1075`. `entity_create_line` forwards a documented `linetype` param (`server.py:446`; a prompt even says "use entity_create_line with CENTER linetype"), but `_BUILTIN_LINETYPES` is only `{continuous, bylayer, byblock}`, so HIDDEN/CENTER fall through to Continuous.
|
||||
- **Fix:** Call `_ensure_linetype_loaded(self._require_doc(), linetype)` before the assignment.
|
||||
|
||||
### R26 — ezdxf `view_zoom_window` is a no-op but reports success; `view_zoom_and_screenshot` ignores the window region
|
||||
- **Severity:** LOW
|
||||
- **Location:** `backends/ezdxf_backend.py:1395-1399` (zoom no-ops), `1401-1425` (screenshot ignores window); `server.py:1887-1893`
|
||||
- **Description:** ezdxf zoom ops return `{"ok": True, "message": …}` while doing nothing. `view_zoom_and_screenshot` passes window coords to the no-op, then `view_screenshot` renders the entire modelspace via `draw_layout(msp)` with `bbox_inches="tight"`, ignoring `x1/y1/x2/y2` — so the image frames the whole drawing despite the docstring promising a window.
|
||||
- **Fix:** Store a pending crop region in `view_zoom_window` and apply via `ax.set_xlim/set_ylim` (drop `bbox_inches="tight"` when cropping); or return `ok:False` for ezdxf window zoom so success no longer implies framing.
|
||||
|
||||
### R28 — COM screenshot HWND via title-substring `EnumWindows` instead of `AcadApplication.HWND`
|
||||
- **Severity:** LOW
|
||||
- **Location:** `backends/com_backend.py:284-301` (`:292`)
|
||||
- **Description:** `_find_autocad_hwnd` picks the first visible top-level window whose caption contains `"AutoCAD"` — a palette, About dialog, or Start tab can win (EnumWindows ~ top-Z first). The COM app object's `.HWND` (the real main frame) is unused.
|
||||
- **Fix:** Use `int(_acad_app().HWND)`; keep EnumWindows only as a fallback.
|
||||
|
||||
### R21 — Gear flank/root junction is a straight radial segment, not a fillet; radial discontinuity when `root_r >= base_r`
|
||||
- **Severity:** LOW
|
||||
- **Location:** `engineering/gear.py:40-50` (flank), `163-190` (root-arc stitch)
|
||||
- **Description:** `generate_involute_flank` prepends `(root_r, 0.0)` while the involute starts at `(base_r, 0.0)` — both on y=0, so root→base is a straight radial segment, not a trochoid. When `root_r >= base_r` (z≥42 @20°, z≥27 @25°, all within the teeth≥6 API limit) no root point is added but the root arc is drawn at `root_r`, giving a radial jump in the closed polyline (verified m=2,z=60: 1.118 mm). (The teeth≤2 degenerate case is unreachable — tools enforce teeth≥6.)
|
||||
- **Fix:** Anchor the flank/root junction at `root_r` (radially extend the first involute point, or use a true trochoid below the base circle); skip the root arc when `root_r >= base_r`.
|
||||
|
||||
### N7 — `point_from_snap` reads ARC angles via bracket indexing → opaque `KeyError` instead of the function's standard `RuntimeError`
|
||||
- **Severity:** LOW
|
||||
- **Location:** `backends/base.py:629-630, 646-647`
|
||||
- **Description:** ARC `"end"`/`"mid"` branches use `p["start_angle"]`/`p["end_angle"]` direct subscripting, while every other geometry access uses `p.get(...)` + descriptive `RuntimeError`. If a backend's extraction failed (broad try/except swallows at `ezdxf:170`/`com:245`), this raises an opaque `KeyError` rather than the consistent error.
|
||||
- **Fix:** Use `p.get(...)` with the same None-check `RuntimeError` pattern as `_line_pts`/`_circle_geom`.
|
||||
|
||||
### N8 — PlanSpec stored in `_plan_spec` is never read; `drawing_critique` does not "replay" it as documented
|
||||
- **Severity:** LOW
|
||||
- **Location:** write `backends/base.py:574`; never read in `engineering/critique.py` `run_critique` (`234-251`); docstrings `base.py:563`, `server.py:2351`
|
||||
- **Description:** `drawing_plan` stores `self._plan_spec = plan`; docstrings claim it's "held by the backend and replayed by drawing_critique." But `run_critique` and all `_check_*` never reference `_plan_spec`/`.intent`/`.scale`/`.view_count`/`.dim_style` (grep: written once, read nowhere). The committed plan is dead state and the "replay" contract is unmet.
|
||||
- **Fix:** Implement plan-vs-drawing checks that read `backend._plan_spec` (e.g. view_count vs detected views, dim_style vs dims present), or correct the docstrings to stop claiming replay.
|
||||
|
||||
### N9 — `view_zoom_extents`/`view_zoom_window` return different payload shapes (ezdxf adds `message`, COM omits it)
|
||||
- **Severity:** LOW
|
||||
- **Location:** `backends/ezdxf_backend.py:1395-1399` vs `backends/com_backend.py:1372-1385`
|
||||
- **Description:** ezdxf returns `{"ok": True, "message": …}`; COM returns `{"ok": True}` with no `message`. Same MCP tool, two shapes — a client keying on `result["message"]` works on ezdxf and `KeyError`s/None on COM.
|
||||
- **Fix:** Return identical keys from both backends (always include `message` + an `applied` bool).
|
||||
|
||||
---
|
||||
|
||||
## SMELL
|
||||
|
||||
### R17 — COM `_safe_send_command` polling holds the lone STA thread; inner `deadline_s` decoupled from outer `com_call_timeout`
|
||||
- **Severity:** SMELL (downgraded from MED — functionally correct; inner 8s deadline is tighter than the 60s outer net, 50 ms sleep is a yielding wait, and holding the STA thread is unavoidable under STA)
|
||||
- **Location:** `backends/com_backend.py:1498-1542`; `config.py:38`
|
||||
- **Fix (optional):** Tie `deadline_s = min(8, com_call_timeout*0.8)` so the two stay consistent if the env var changes.
|
||||
|
||||
### R29 — Broad `except Exception` guards around optional COM reads
|
||||
- **Severity:** SMELL (downgraded — adjacent unguarded calls re-raise genuine disconnects into `_run`; most sites log rather than silently pass; reconnect is not bypassed)
|
||||
- **Location:** `backends/com_backend.py:200, 245, 251, 470, 1447, 1512-1548` (~30 sites)
|
||||
- **Fix:** Narrow to `except (pywintypes.com_error, AttributeError)` so genuine logic bugs surface.
|
||||
|
||||
### S5 — Two parallel, conflicting layer systems
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `server.py:1647-1688` (`_LAYER_TEMPLATES`) vs `engineering/layers.py:14-69` (`ENGINEERING_LAYERS`/`LAYER_SET_REGISTRY`)
|
||||
- **Description:** `_LAYER_TEMPLATES` (architectural/mechanical/…) uses hundredths-int lineweights, all-Continuous linetypes, no CONSTRUCTION layer; `engineering/layers.py` (mech/pid/iso13567) uses mm-floats, real HIDDEN/CENTER, ISO-128 lineweights, CONSTRUCTION. The overlapping mechanical/mech sets *conflict* (HIDDEN/CENTER colors swapped), and `_LAYER_TEMPLATES` violates the ISO-128 discipline `drawing_critique` enforces.
|
||||
- **Fix:** Retire `_LAYER_TEMPLATES`/`template_apply_layers` in favor of `LAYER_SET_REGISTRY` (or map legacy keys onto it) for one source of truth.
|
||||
|
||||
### S3 — `BlockInfo.description` always empty
|
||||
- **Severity:** SMELL
|
||||
- **Location:** declared `backends/base.py:71`; never set in `ezdxf_backend.py:1196-1202` / `com_backend.py:1186-1192`
|
||||
- **Fix:** Populate `description=blk.block.dxf.get("description", "")` (ezdxf) + COM equivalent, or drop the field.
|
||||
|
||||
### S1 — `engineering/section.py` helpers (`cutting_plane_line`, `apply_section_hatch`) are dead
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `engineering/section.py:12, 65`; only re-exported (`__init__.py:35, 65-66`), never called (`gear_draw_section_aa` builds its hatch inline).
|
||||
- **Fix:** Wire a `section_*` MCP tool around them, or delete the module + exports.
|
||||
|
||||
### S2 — `generate_tooth_profile` is dead (superseded by `generate_full_gear_outline`)
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `engineering/gear.py:74`; exported `__init__.py:9, 59`; duplicates ~80% of the outline math (unused local `t_pitch` at `:94`).
|
||||
- **Fix:** Delete and remove import/`__all__` entries, or wire into a draw path.
|
||||
|
||||
### S4 — Stale "Agent B still in flight" build-seam comment + now-vestigial import guards
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `engineering/__init__.py:37-39` (comment), `40-51` (try/except guards)
|
||||
- **Description:** Both `titleblock.py` and `validator.py` now exist and import cleanly, so the guard's rationale is stale. (Related: **R14** — when a guarded symbol *is* None the tools call it and raise an opaque `TypeError`; currently unreachable since modules exist.)
|
||||
- **Fix:** Delete the build-seam comment; either drop the guards (import directly) or have the tools raise `ToolError("engineering module unavailable: …")` when a symbol is None instead of calling it.
|
||||
|
||||
### N10 — `CommandResult` dataclass + `to_dict` never instantiated
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `backends/base.py:26-38`; exported `backends/__init__.py:2, 8`
|
||||
- **Description:** Defined and re-exported, but no constructor call exists outside docs; every command return uses a plain dict. CLAUDE.md/README still list it as a shared dataclass.
|
||||
- **Fix:** Delete it + its export + doc refs, or actually adopt it as the command return type.
|
||||
|
||||
### N11 — `_ACI_NAMES` color-name table defined but never referenced
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `backends/ezdxf_backend.py:54-57`
|
||||
- **Fix:** Delete, or wire into `_entity_info_dxf`/`_layer_info_dxf` as a `color_name` field.
|
||||
|
||||
### N12 — `AutoCADBackend.set_layer_active` alias has zero callers
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `backends/base.py:910-912`
|
||||
- **Fix:** Remove; callers use `layer_set_current` directly.
|
||||
|
||||
### N13 — `AutoCADBackend.ensure_linetypes` concrete helper is dead (superseded by `engineering.layers.ensure_standard_linetypes`)
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `backends/base.py:914-934`
|
||||
- **Description:** Concrete ABC method with no callers; the real bootstrap path (`server.py:288`, `layers.py:110`) calls the module-level `ensure_standard_linetypes`, which reimplements the same already_loaded/loaded/failed loop.
|
||||
- **Fix:** Delete it, or refactor `ensure_standard_linetypes` to delegate to it (single owner).
|
||||
|
||||
### N14 — COM `self._batch_mode` instance attr written but never read (`_regen` consults only `_COM_STATE`)
|
||||
- **Severity:** SMELL
|
||||
- **Location:** `backends/com_backend.py:363, 889, 907, 916, 933` vs `_regen` `180-187`
|
||||
- **Description:** `_regen` guards on `_COM_STATE.get("batch_mode")` only. The array methods set *both* `self._batch_mode` and `_COM_STATE['batch_mode']`; the instance attr has no readers and can desync.
|
||||
- **Fix:** Remove the redundant `self._batch_mode` assignments (keep the dict flag), or make `_regen` read the instance attr and drop the dict — one source of truth.
|
||||
|
||||
### R27 — Validator step 5 uses substring matching on all TEXT-layer text
|
||||
- **Severity:** SMELL (downgraded from LOW — gated by `if helix:`, only runs when caller declares `helix_angle`; flagging a SPUR title on a declared-helical part is intended; only the over-broad substring scan is a smell)
|
||||
- **Location:** `engineering/validator.py:159-181` (`:168`, `:175`)
|
||||
- **Fix:** Restrict the scan to title-block text and use word-boundary regex (`\bSPUR\b`/`\bHELICAL\b`) instead of `in`.
|
||||
|
||||
### R25-residual — `block_find_references` INSERT cap
|
||||
- **Severity:** SMELL (memory-bug claim REFUTED — `entity_list` defaults to `limit=200`, hard-capped; see appendix)
|
||||
- **Location:** `server.py:1368-1369`
|
||||
- **Fix (optional):** Warn when INSERT count hits the 200 cap to avoid silently under-reporting references in large drawings.
|
||||
|
||||
---
|
||||
|
||||
## (a) Count Summary by Severity
|
||||
|
||||
| Severity | Count | IDs |
|
||||
|---|---|---|
|
||||
| CRITICAL | 0 | — |
|
||||
| HIGH | 2 | N1, N2 |
|
||||
| MED | 13 | R4, R5, R6, R9, R12, R13, R15, R18, R19, R22, N3, N4, N5, N6 *(14 listed — see note)* |
|
||||
| LOW | 9 | R16, R20, R23, R24, R26, R28, R21, N7, N8, N9 *(10 listed — see note)* |
|
||||
| SMELL | 13 | R17, R29, R25, R27, S1, S2, S3, S4, S5, N10, N11, N12, N13, N14 *(14 — see note)* |
|
||||
|
||||
> Note: counts reflect merges (R10+R11 → N2; R23 split across N6+R23; R13 absorbs two fresh-sweep offset findings; R10 residual folded into N2). Exact kept-finding totals: **HIGH 2, MED 14, LOW 10, SMELL 14 = 40 kept**; 3 dropped (refuted/already-fixed appendix below).
|
||||
|
||||
---
|
||||
|
||||
## (b) Confirmed P1 Shortlist — highest-leverage fixes to tackle next
|
||||
|
||||
1. **N1 — COM hatch associativity/delete bug** (`com_backend.py:659-673`). Flip `AddHatch(…, True)` → `False`. One-line fix, prevents corrupt/invalid hatches in live AutoCAD.
|
||||
2. **N2 — unify `drawing_save_as` format + derive from extension in `drawing_finalize`** (`base.py:158`, `ezdxf_backend.py:346`, `com_backend.py:516`, `server.py:2299`). Fixes silent DXF-as-DWG mislabeling and cross-backend format divergence on the primary output path.
|
||||
3. **R19 — fix the two ezdxf dimension calls + make xfail strict** (`ezdxf_backend.py:696, 711-715`; `tests/test_dimensions.py:16,23`). Two advertised tools ship broken; cheap, high-visibility.
|
||||
4. **R4 — make `dim_overlap` actually run** (`critique.py:199-201` + extractors). Restores a premium-workflow safety check that is currently a guaranteed no-op.
|
||||
5. **R18 — surface bootstrap failure in `drawing_new`** (`server.py:282-293`). Prevents silent production-precondition violations (geometry on missing layers).
|
||||
6. **N4 — parameterize `dimension_auto` / construction layers by active layer set** (`base.py:822/836/845/852, 743, 754`). Fixes ISO-13567 dims/construction landing on wrong auto-created layers.
|
||||
7. **R9 — fix full-circle polar array overlap + COM divergence** (`ezdxf_backend.py:947`). Eliminates duplicate-entity output and a backend behavior split.
|
||||
8. **R6 — add a mocked COM unit-test suite** (`tests/test_com_backend.py`). Unlocks regression safety for 1651 lines / 85 methods that currently can't be caught on CI — force-multiplier for every other COM fix above.
|
||||
9. **R5 — bound the COM timeout thread/apartment leak** (`com_backend.py:416-434`). Prevents long-session resource exhaustion in live AutoCAD.
|
||||
10. **R13/N5 — `entity_offset` side params + property-shape parity** (`ezdxf_backend.py:869-909`, `com_backend.py:861-875`; props extractors). Removes two dead documented params and the cross-backend `properties` contract drift that breaks downstream clients.
|
||||
|
||||
---
|
||||
|
||||
## (c) Refuted / Already-Fixed Appendix (checked, not actioned)
|
||||
|
||||
- **R1 — premium tools missing on COM:** ALREADY FIXED in `cd6329a`. The 8 premium meta-tools are now concrete methods on `AutoCADBackend` (`backends/base.py`); only `_create_xline` is backend-specific.
|
||||
- **R2 — `int(lineweight)` truncation:** ALREADY FIXED in `cd6329a` via `normalize_lineweight()` in `base.py` (reconciles mm-float vs hundredths-int).
|
||||
- **R3 — `iso128` critique no-op:** ALREADY FIXED in `cd6329a`.
|
||||
- **R8 — _(no verification payload supplied; not assessed)._** Listed only for register completeness.
|
||||
- **R14 — engineering import-guard `TypeError`:** PARTIAL / **not reachable**. Both `titleblock.py` and `validator.py` exist and import cleanly, so the `None`-fallback branch is never taken in the shipped repo. The vestigial guards/comment are carried as **S4** (SMELL).
|
||||
- **R25 — `block_find_references` unbounded INSERTs + KeyError:** REFUTED. `entity_list` defaults to `limit=200` (hard-capped at all three layers), the opposite of "unbounded"; `e.properties.get("block_name")` cannot `KeyError`. Optional cap-warning carried as a SMELL above.
|
||||
- **R29 — COM broad `except Exception` masks disconnects:** REFUTED as stated (reconnect not bypassed; most sites log). Retained as SMELL (narrow the guard types).
|
||||
- **R17 — `_safe_send_command` deadline decoupling as a defect:** the defect framing is REFUTED (inner deadline is the tighter, primary one); retained as SMELL.
|
||||
- **S4 (`Task #7/#8` phrasing):** the specific cited phrase is REFUTED (the comment says "Agent B … still in flight," not "Task #7/#8"); the substantive build-seam-comment smell STANDS and is kept as S4.
|
||||
@@ -0,0 +1,146 @@
|
||||
<!-- Generated by the IDEAS/roadmap workflow (wf_02a3262e-ddd), 2026-06-18. 19 ideas scored, 9 stress-tested. Pairs with docs/analysis/01-DIAGNOSTICS.md. -->
|
||||
|
||||
# AutoCAD MCP Pro — Evidence-Based Roadmap
|
||||
|
||||
> Mission: become the best AutoCAD MCP for LLM-driven drawing. The durable moat is the **standards-validation gate** (no competing CAD MCP — build123d / FreeCAD / Blender / Rhino / Onshape — ships any design-rule validation). Everything below is sequenced to widen that moat or to remove the silent no-ops that make it lie.
|
||||
|
||||
Priority signal = `(impact + differentiation + evidence_strength) / effort`, computed on **post-challenge adjusted** scores, then adjusted by judgment (quick wins and dependency order). DEFER → Backlog; RESCOPE → adjusted scope used; KEEP → as scored.
|
||||
|
||||
---
|
||||
|
||||
## Scoring table (all ideas, adjusted, sorted by priority signal)
|
||||
|
||||
| ID | Title | Imp | Eff | Diff | Ev | Signal | Verdict | Area |
|
||||
|----|-------|-----|-----|------|----|--------|---------|------|
|
||||
| **I9** | Expand deterministic geometry surface | 3 | 2 | 3 | 5 | **5.5** | KEEP→rescoped* | determinism |
|
||||
| **I3** | ProCAD pre-plan clarification pass | 3 | 2 | 4 | 4 | **5.5** | RESCOPE | closed-loop |
|
||||
| **I16** | Correctness + hygiene bundle (DIAGNOSTICS) | 3 | 2 | 2 | 5 | **5.0** | KEEP | hygiene |
|
||||
| **I6** | GD&T primitive (ISO 1101 / Y14.5) | 5 | 3 | 5 | 3 | **4.3** | KEEP (impact↑) | iso-production |
|
||||
| **I14** | NL→primitive grounding (trim/fillet/chamfer) | 3 | 3 | 3 | 4 | **3.3** | RESCOPE | determinism |
|
||||
| **I15** | Staged validator (code→geometric→rubric) | 5 | 3 | 5 | 5 | **5.0** | RESCOPE | metrics |
|
||||
| **I12** | Authenticated remote HTTP | 2 | 1 | 2 | 2 | **6.0** | KEEP (quick win) | infra |
|
||||
| **I4** | Scalar drawing-score / Invalidity-Ratio | 3 | 2 | 3 | 4 | **5.0** | KEEP | metrics |
|
||||
| **I2** | Visual critique / render as ImageContent | 3 | 2 | 4 | 5 | **6.0** | RESCOPE (cheap half) | closed-loop |
|
||||
| **I18** | Real drawing repair (recover + Auditor.fix) | 2 | 2 | 1 | 3 | **3.0** | KEEP | robustness |
|
||||
| **I1** | Routed iterative Refiner loop | 4 | 4 | 5 | 5 | **3.5** | RESCOPE | closed-loop |
|
||||
| **I5** | ISO 129 tolerances on dimensions | 4 | 4 | 3 | 5 | **3.0** | RESCOPE | iso-production |
|
||||
| **I8** | ISO-129 DimStyle + MLEADER | 3 | 3 | 2 | 3 | **2.7** | as scored | iso-production |
|
||||
| **I19** | COM test harness (mocked) + cov gate | 2 | 2 | 1 | 3 | **3.0** | KEEP | hygiene |
|
||||
| **I7** | Real plotted sheets + real DWG | 3 | 3 | 3 | 3 | **3.0** | RESCOPE (cheap half) | iso-production |
|
||||
| **I17** | Fast selection via native query | 2 | 3 | 1 | 2 | **1.7** | ezdxf half only | perf |
|
||||
| **I13** | Standard-parts / symbol library | 3 | 4 | 3 | 2 | **2.0** | RESCOPE | breadth |
|
||||
| **I11** | Parametric editability / conv. edit | 3 | 4 | 3 | 2 | **2.0** | as scored | breadth |
|
||||
| **I10** | 3D solids tier (or fix false claim) | 2 | 4 | 2 | 2 | **1.5** | split | breadth |
|
||||
|
||||
\* I9/I3 share the top signal; I9 edges first because it is zero-risk and directly converts the GeoGramBench failure mode (<50% program-to-geometry) into deterministic tool calls.
|
||||
|
||||
---
|
||||
|
||||
## Sprint 1 — Quick wins & correctness
|
||||
|
||||
Foundation: the closed-loop moat only pays off if the validator/critique signals are not silent no-ops. Land the cheap, CI-verifiable correctness fixes and the highest-signal determinism wins first.
|
||||
|
||||
### I16 — Correctness + hygiene bundle (DIAGNOSTICS fixes) · imp 3 / eff 2 / diff 2
|
||||
**Why now:** Every premium-workflow signal that the moat depends on is currently degraded by confirmed defects (fake DWG, broken aligned/angular dims, polar-array dup, silent bootstrap). Iteration evidence (Self-Improving CAD +13.4pp/round) only pays off if signals aren't dead. **Depends on:** land alongside I19 (COM-only N1/R5 need a harness).
|
||||
**First step:** One PR for the CI-verifiable wins: fix R19 (`ezdxf_backend.py:696` `dist`→positional `distance`; `:711` `add_angular_dim_2l(base=…, line1=…, line2=…, location=(tx,ty))`), flip `tests/test_dimensions.py:16,23` to `xfail(strict=True)`, fix R9 divisor (`:947`), R12 `location=(tx,ty)` (`:715`), R13 offset side via cross-product sign; then `ruff check --fix .` (41 auto) + 6 manual. Verified: ezdxf `drawing_save_as` ignores `fmt` and returns `{"format": fmt}` while writing DXF bytes (`ezdxf_backend.py:346-353`) — make it derive fmt from extension and refuse to mislabel.
|
||||
|
||||
### I9 — Expand deterministic geometry surface · imp 3 / eff 2 / diff 3
|
||||
**Why now:** Highest priority signal. `point_from_snap` is a **concrete** base method sourcing from the shared `EntityInfo.properties` contract (verified at `base.py:585`), so new snaps are pure math in one place, zero per-backend code, free on COM+ezdxf. Finishes the pre-committed `int` stub (`plan_spec.py:24`). **Depends on:** none.
|
||||
**First step:** Implement the deferred `point_intersection` (line-line / line-circle / circle-circle, ref-point disambiguation) and `point_tangent` (external-point-to-circle — highest leverage). Demote/drop `point_offset` (trivial vector add). Either cover LWPOLYLINE or document the gap explicitly. Dual-backend tests in `tests/test_meta_tools.py`.
|
||||
|
||||
### I12 — Authenticated remote HTTP · imp 2 / eff 1 / diff 2 — **QUICK WIN / latent security bug**
|
||||
**Why now:** Confirmed: no `auth=` arg and no `TokenVerifier` anywhere (grep returned nothing). `_validate_http_bind` promises bearer-token auth but it is **never enforced** — a remote operator who sets `ALLOW_REMOTE_HTTP`+`MCP_AUTH_TOKEN` gets an unauthenticated server that believes it is authenticated. FastMCP 3.0.1 ships `StaticTokenVerifier`. ~10 lines.
|
||||
**First step:** Build `_auth = StaticTokenVerifier(tokens={settings.mcp_auth_token: {...}}) if settings.mcp_auth_token else None`, pass `auth=_auth` into `FastMCP(...)` at `server.py:153`; add `tests/test_http_auth.py` (401 on bad/missing token, pass on correct).
|
||||
|
||||
### I12-adjacent quick win — I2 cheap half (render as ImageContent) · imp 3 / eff ~1 / diff 4
|
||||
**Why now:** `fastmcp Image` is already imported/returned by `view_screenshot` (`server.py:1846+`); `drawing_finalize` writes screenshot bytes to disk and discards them. Returning the existing PNG bytes as `ImageContent` lets the already-multimodal driving agent (Claude) see the render — captures the CADSmith "render is a first-class signal" win with **no model bundling, no torch, no [vision] extra**.
|
||||
**First step:** Add `return_image`/`include_render` to `drawing_finalize` (or a thin `visual_inspect` tool) that wraps existing bytes in `Image`. Keep the deterministic critique gate pure — the visual signal is advisory, **not** a `CritiqueFocus` member. Defer the local-VLM judge and OCR dim round-trip.
|
||||
|
||||
---
|
||||
|
||||
## Phase A — Own the moat (closed-loop validation)
|
||||
|
||||
This is the differentiation core. Sequence: scalar metric → staged unified validator → clarification gate → refiner loop. The metric makes the loop measurable; the unified validator fixes a real disconnect; the refiner acts on it.
|
||||
|
||||
### I4 — Scalar drawing-score / Invalidity-Ratio · imp 3 / eff 2 / diff 3
|
||||
**Why now:** No quantitative metric exists today; `drawing_critique` returns a list, `validator.run` returns severity counts only. MUSE/CadBench grade Invalidity-Ratio, not shape. A scalar is the objective function that makes the repair loop measurable and the smoke test regression-trackable. **Depends on:** sequence after I16 (so the score isn't averaged over dead checks like R4 dim_overlap).
|
||||
**First step:** Add `engineering/scoring.py::score_findings(severity_counts)` → `score = 100 − (errors·Wₑ + warnings·Wᵥᵥ + info·Wᵢ)` clamped, plus `invalidity_ratio = errors/max(1, findings)`. Consume from both `validator.py` and `critique.py`; surface in `drawing_finalize` payload. Document weights as a convention, not ground truth.
|
||||
|
||||
### I15 — Staged validator (code→geometric→rubric) · imp 5 / eff 3 / diff 5 — RESCOPE
|
||||
**Why now:** Strongest hidden find: `drawing_finalize` (`server.py:2311`) calls **only** `DrawingValidator.run()` and **never** `run_critique` — the 6 premium critique focuses are not part of the finalize gate. Unifying validator's 8 steps + critique's 6 focuses into one staged protocol fixes a real architectural disconnect, not just cosmetics. **Depends on:** the no-op critique fixes (R4/R12/R19 in I16) must land first or the staged scalar lies. Drop R27 (separate SMELL); defer the VLM rubric stage to I2's deferred half.
|
||||
**First step:** Define `StagedReport` (stages: code/geometric) with `{rule_id, stage, severity, passed, handles, detail}` + the I4 scalar; refactor `DrawingValidator.run` to tag findings with stage and reuse `_FOCUS_DISPATCH` for the geometric stage; wire **both** into `drawing_finalize`. Assert the scalar in `scripts/smoke_premium.py`.
|
||||
|
||||
### I3 — ProCAD pre-plan clarification pass · imp 3 / eff 2 / diff 4 — RESCOPE
|
||||
**Why now:** ProCAD's one clarification round cut invalidity 14.6%→0.9% — attacks "garbage in" before any geometry, which no downstream critique recovers. `drawing_plan` is a thin concrete method building a `PlanSpec` (`base.py:553`). **Depends on:** none, but deep material/tolerance NL-extraction should wait for the I5 tolerance model. **Rescope:** ship cheap robust checks only — PlanSpec has no structured tolerance/material fields, so deep NL-parsing balloons effort.
|
||||
**First step:** `engineering/plan_spec.py::detect_ambiguities(plan)` mirroring `_FOCUS_DISPATCH`: rule 1 = `view_count` vs intent keywords (section/orthographic/3-view); rule 2 = scale-vs-sheet sanity; rule 3 = ordinate dim_style needs a datum note. Surface advisory `clarifications` in the server wrapper return.
|
||||
|
||||
### I1 — Routed iterative Refiner loop · imp 4 / eff 4 / diff 5 — RESCOPE
|
||||
**Why now:** The validation-gate-as-repair-loop is the moat no competitor has, directly operationalizing CADSmith Refiner (38x) and FEA-agent (+13.4pp/round). All repair primitives exist on both backends with transaction rollback. **Depends on:** I15 (unified staged report) and I16 (R4/R19). **Rescope to honest v1:** auto-repair only the 3 clean deterministic focuses + iso128-snap; route `untrimmed_corner` and `dim_overlap` to the LLM as **hints**, not auto-fixes.
|
||||
**First step:** `engineering/refine.py` with a `REPAIR_HINTS` registry mapping each `CritiqueFocus` to (a) a deterministic repair coroutine and (b) a fallback hint. Implement the 4 trivial focuses (`construction_left`→`construction_clear`, `duplicate_entities`→`entity_delete(handles[1])`, `layer_color`→`layer_modify(color=detail.expected)`, `iso128`→snap-to-nearest-ISO). Then `drawing_refine(max_rounds=4)` looping critique→repair→re-critique. **Critical blocker (R22):** the ezdxf `_undo_stack` is shared between user undo and transactions with no txn-id isolation (`system_status` even reports `len(_undo_stack)` as transaction_depth) — add a dedicated txn list/guard before the rollback cycle, or it corrupts undo state.
|
||||
|
||||
---
|
||||
|
||||
## Phase B — Production ISO parity
|
||||
|
||||
Make output actually manufacturable. ISO 129 tolerances are the single biggest functional gap; GD&T is the highest-differentiation breadth item.
|
||||
|
||||
### I6 — GD&T primitive (ISO 1101 / Y14.5) · imp 5 / eff 3 / diff 5 — KEEP (impact↑)
|
||||
**Why now:** Maximal differentiation — 2D GD&T is absent from every surveyed text-to-CAD product and every competing MCP. Pure composition over existing primitives (the `titleblock_apply_iso_a3` pattern, `server.py:2248`) — no ABC/dual-backend additions. Impact raised to 5: bundle a thin tolerance-text option and one GD&T critique check (datum letter in an FCF must have a matching datum feature) to convert it from "draws a symbol" to "validates a standard." **Depends on:** none (composes cleanly).
|
||||
**First step:** `engineering/gdt.py` with the 14 ISO 1101 characteristic glyphs + Ⓜ/Ⓛ/Ⓢ modifiers (unicode/MTEXT codes), then `draw_feature_control_frame(...)` composing the frame from `entity_create_line` + `entity_create_text` on DIM layer. Register one `gd_frame` `@mcp.tool`. **Design constraint:** avoid ezdxf's native Tolerance/FCF entity (renders blank through matplotlib) — compose from lines+text. Smoke-test glyph rendering via screenshot.
|
||||
|
||||
### I5 — ISO 129 tolerances on dimensions · imp 4 / eff 4 / diff 3 — RESCOPE
|
||||
**Why now:** A drawing without ±/limit/fit callouts is not manufacturable — the dossier's "biggest functional gap." Empirically verified ezdxf 1.4.3 renders sym/limit/text overrides via override dict. **Depends on:** R19 fix (I16) is a soft prerequisite — aligned/angular dims are currently broken, so don't bolt tolerances onto them yet. **Rescope:** ezdxf-first, CI-testable subset; defer aligned/angular tolerances behind R19; COM tolerance rendering best-effort/untested until I19.
|
||||
**First step:** Add `tol_upper/tol_lower/tol_mode/fit/text_override` to `dimension_linear/radius/diameter` in `base.py`; implement in ezdxf via override dict (sym/limit → DIMTOL/DIMLIM + DIMTP/DIMTM; fit → `engineering/iso286_fits.py` nominal-band×grade table — the main authored-data cost). Test asserts the DIMENSION carries DIMTP/DIMTM and reads e.g. `20 +0.021/0`.
|
||||
|
||||
### I8 — ISO-129 DimStyle + MLEADER · imp 3 / eff 3 / diff 2 — as scored
|
||||
**Why now:** Every dim today inherits the default "Standard" style (no DIMTXT/DIMASZ/precision). Natural sibling to `drawing_apply_iso_layers`. Low differentiation (table-stakes) — sequence after I5/I6. **Depends on:** I5 (share the dimstyle plumbing).
|
||||
**First step:** `engineering/layers.py::apply_iso_dimstyle(backend)` → `doc.dimstyles.new("ISO-25", …)`; thread `dimstyle="ISO-25"` through ezdxf dim methods; wire into `drawing_apply_iso_layers`. Defer COM `DimStyles.Add` and the MLEADER tool to follow-ups.
|
||||
|
||||
### I7 — Cheap half: fmt-from-extension + true ISO-128 lineweights · imp 3 / eff 3 / diff 3 — RESCOPE
|
||||
**Why now:** Two isolated wins. (1) Fake-DWG is ezdxf-only (COM already writes real DWG via `SaveAs(path,12)`); (2) ISO-128 true lineweights via ezdxf `Configuration` (`lineweight_policy`) on the render path is the genuinely differentiating piece and feeds the render/critique signal. **Defer** the from-scratch paperspace/viewport/Plot pipeline (multi-day, CI-unverifiable, `odafc` needs the ODA binary — confirmed not on PATH) to the backlog. **Depends on:** none.
|
||||
**First step:** Wire `Configuration(lineweight_policy=…)` into the PDF/screenshot Frontend; unify the save_as fmt default and fail loud when ezdxf can't write real DWG (use `odafc.export_dwg` if present, else raise — never mislabel).
|
||||
|
||||
---
|
||||
|
||||
## Phase C — Breadth & determinism extras
|
||||
|
||||
Catch-up capability and convenience. Lower priority signals; do not displace the moat work.
|
||||
|
||||
### I14 — NL→primitive grounding for trim/fillet/chamfer · imp 3 / eff 3 / diff 3 — RESCOPE
|
||||
**Why now:** Repair-side companion to the `untrimmed_corner` detector; attacks FutureCAD primitive-grounding + GeoGramBench. **Rescope:** "which corner" is net-new work — `analysis_bounding_box` returns only a global bbox, so corner→handle mapping re-runs O(n²) endpoint clustering. Ship ezdxf path with bbox-quadrant corner naming that **errors with candidates when ambiguous** instead of guessing; defer COM until I19 lands COM tests. **Depends on:** I9 (intersection math), I19 (COM verification).
|
||||
|
||||
### I18 — Real drawing repair (recover + Auditor.fix) · imp 2 / eff 2 / diff 1 — KEEP
|
||||
**Why now:** Confirmed honesty bug: `drawing_audit` returns only `auditor.errors`, ignores `auditor.fixes`, while the docstring claims it "fixes errors"; `drawing_open` uses bare `readfile` with no `recover.readfile` fallback. Near-free robustness win.
|
||||
**First step:** Return `errors`+`fixes`+`fixed_count` and `_mark_dirty()` when fixes applied; wrap `readfile` with a `recover.readfile` fallback.
|
||||
|
||||
### I19 — COM test harness (mocked) + scoped coverage gate · imp 2 / eff 2 / diff 1 — KEEP
|
||||
**Why now:** R6 confirmed — zero tests touch `ComBackend`; CI is ubuntu-only so 1651 lines never run. `normalize_lineweight` (the P0 anti-truncation fix, pure/OS-agnostic) has no regression guard. Unblocks COM verification for I14/I5/I16. **Depends on:** none; should land **before/with** I16's COM-only items (N1/R5).
|
||||
**First step:** `tests/test_com_backend.py` with a `sys.modules` fake-COM shim; parametrized `normalize_lineweight` table tests + `_safe_send_command` CMDACTIVE/timeout/var-restore tests. Scope `--cov-fail-under` to `backends.base` + tested modules, not repo-wide.
|
||||
|
||||
### Backlog / Later (!)
|
||||
|
||||
- **I17** — Fast selection: do the **ezdxf-only** half (`query()`/`groupby()` one-liners, zero-risk, headless-testable); defer the COM `SelectAll` rewrite (CI-unverifiable, finicky compound filters). Not a moat-widener.
|
||||
- **I13** — Standard-parts library: ship the cheap `library_insert` wrapper + a few parametric ISO bolt block defs; the real cost (ISO-correct fastener + P&ID symbol authoring) is large ongoing work. Defer dynamic-block driving (COM-only, breaks dual-backend parity).
|
||||
- **I11** — Parametric editability: ship single-entity `entity_edit_geometry` (set radius/endpoints) first; true re-drive needs a constraint graph the architecture has no scaffold for. Catch-up, not moat.
|
||||
- **I10** — 3D solids: **do the near-zero-effort honesty fix now** (correct the false `"all_entity_types"` capability at `com_backend.py:1444`). Defer the COM-only solids tier; ezdxf has no ACIS kernel so it breaks dual-backend parity, and all cited research targets 2D fidelity.
|
||||
- **I7 expensive half** — paperspace/viewport/Plot pipeline + bundled-VLM judge.
|
||||
|
||||
---
|
||||
|
||||
## Recommended next 5 work items
|
||||
|
||||
1. **I16 (Correctness bundle) + I19 (COM harness) together.** Highest evidence strength; everything in Phase A is built on signals that are currently silent no-ops (R4 dim_overlap, R19 broken dims, fake DWG). The COM harness lands with it so the COM-only fixes (N1, R5) and `normalize_lineweight` are actually guarded. Foundation for the entire moat.
|
||||
2. **I9 (Deterministic geometry surface).** Top priority signal, zero risk, one-place pure math. Directly converts the GeoGramBench <50% program-to-geometry failure into deterministic tool calls — the cheapest moat-adjacent win available.
|
||||
3. **I12 (HTTP auth) + I2-cheap (render as ImageContent).** Two quick wins. I12 closes a real false-security gap (~10 lines, verified no auth wiring exists). I2-cheap captures the CADSmith "render is a first-class signal" win for ~1 day with no model bundling, by returning the already-produced PNG to the multimodal driver.
|
||||
4. **I4 → I15 (Scalar metric → unified staged validator).** I4 gives the objective function; I15 fixes the confirmed disconnect where `drawing_finalize` never runs the premium critique focuses. Together they make the moat measurable, regression-testable, and actually enforced at finalize — the prerequisite for the refiner.
|
||||
5. **I6 (GD&T primitive).** Highest-differentiation feature in the dossier, KEEP at effort 3, impact raised to 5 by bundling a tolerance-text option and one datum-consistency critique. Pure composition, no backend surface change — the single biggest "no competitor has this" capability.
|
||||
|
||||
Sequencing rationale: items 1–4 are explicitly ordered by `depends_on` (signals must be honest before the scalar averages them; the scalar must exist before the refiner can optimize it; R22 txn-isolation gates the refiner's rollback cycle). Item 5 is parallelizable — it composes cleanly and shares no dependency with the closed-loop track, so it can run alongside Phase A.
|
||||
|
||||
---
|
||||
|
||||
## Security note — unverified DIAGNOSTICS items (R7/R8)
|
||||
|
||||
The diagnostics flagged two security items that the security-review subagents **could not verify**: **R7** (input-sanitizer wiring — whether `system_run_command` / `system_run_lisp` user strings are actually routed through a sanitizer before reaching `SendCommand`) and **R8** (absence of adversarial sanitizer tests). These sit on the live-COM command path, which is the highest-blast-radius surface in the repo (arbitrary AutoLISP/command execution) and is exactly the path with **zero COM test coverage** (R6).
|
||||
|
||||
**Recommendation — add a defensive hardening work item (slot in Sprint 1, alongside I19):** (a) trace `system_run_command`/`system_run_lisp` end-to-end and confirm every user-supplied string passes through an explicit allow/deny sanitizer before `_safe_send_command`; if no sanitizer is wired, add one (reject embedded newlines, `\x03`/ESC injection, nested `(command …)` escapes, and `SCRIPT`/`SHELL`/`START` command words). (b) Add adversarial unit tests on the mocked-COM harness from I19 asserting that known injection payloads are rejected before reaching `SendCommand`. This is cheap insurance on the one surface where a prompt-injected agent could execute arbitrary OS-level actions via `SHELL`/`START`, and it composes directly with the I19 fake-COM fixture.
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user