Files
autocad-mcp-pro/docs/analysis/03-AUDIT-VERIFICATION.md
T
Umutcan Edizsalan 2c2fba1459 docs: release v1.1.0 — accurate counts, changelog, audit report
- pyproject: version 1.0.0 -> 1.1.0
- README: 110 tools / 5 resources (was 87 / 6), 318 tests, new "Closed-Loop
  Quality Gate" highlight, Engineering + Premium tool categories, refreshed
  roadmap (1.1 shipped; 1.2 closed-loop moat; 1.3 ISO production).
- CHANGELOG: comprehensive [1.1.0] entry (Added/Changed/Fixed/Security).
- CLAUDE.md: drop removed CommandResult dataclass; ~110 tool count.
- docs/analysis/03-AUDIT-VERIFICATION.md: the verified audit + fix plan.
2026-06-19 08:37:08 +03:00

18 KiB

AUDIT + VERIFICATION — AutoCAD MCP Pro (post Sprint-1)

This audit (a) verifies the 4 Sprint-1 commits, (b) re-confirms which of the 40 baseline diagnostics are still open after those commits, and (c) adds fresh-sweep findings — then sequences everything into a fix plan. Line numbers reflect the current working tree (branch fix/http-auth, 233 tests green, ruff clean).


A. Verdict on the 4 Sprint-1 commits

Commit Item Verdict
2b80d00 I16 N1 COM hatch associativity→False ✅ correct & complete
R9 polar-array 360° divisor ✅ correct (divisor=count on full ring; matches COM total-instance count)
R12 / R19 ezdxf aligned/angular dim API ✅ correct (matches ezdxf 1.4.3 sig; xfail removed, passes strict)
dead-code removal (section.py, generate_tooth_profile, CommandResult, _ACI_NAMES, set_layer_active, ensure_linetypes, _batch_mode) ✅ clean — zero dangling Python refs; only docs still mention them
N12 / N13 / N14 dead helpers ✅ removed
R13 entity_offset side_x/side_y ⚠️ incomplete — honored in ezdxf only; COM still ignores it
R18 drawing_new bootstrap surfacing ⚠️ incomplete — only a total exception → degraded; per-item failed: swallowed → still bootstrap.ok=True
N2 save format ⚠️ partial — drawing_finalize derives fmt from extension ✅, ezdxf refuses dwg ✅, but the standalone drawing_save_as tool still defaults fmt='dwg' and ignores the extension
0f1f051 I9 point_intersection / point_tangent ✅ math fully correct (LINE-LINE, LINE-CIRCLE, CIRCLE-CIRCLE, tangent perpendicularity verified numerically); 13 tests assert real invariants
82c9fe0 I12 StaticTokenVerifier wiring ✅ auth=_auth correctly passed; token never logged; reject works at runtime
Guard enforcement ⚠️ gap — _validate_http_bind runs only in __main__; the documented fastmcp run import path skips it (see NEW-AUTH-1)
f2f3fa6 I19 _safe_send_command coverage ✅ meaningful (var-restore, timeout, new-handle faithful)
normalize_lineweight table ⚠️ omits the ambiguous bare ints 1/2; COM transactions + CMDACTIVE guard still untested

Bottom line: the Sprint-1 fixes that landed are correct where they landed. The recurring defect pattern is one-backend / partial fixes: R13 (ezdxf-only), R18 (total-failure-only), N2 (finalize-only), I12 guard (__main__-only). Cross-backend parity is the theme of the open work.


B. Confirmed open issues (ranked)

Severity recalibrated by impact-on-common-path. ★ = re-confirmed by direct code read in this session.

HIGH

  • NEW-screenshot-1 ★ — view_screenshot / drawing_export_pdf build a matplotlib.pyplot figure inside an asyncio.to_thread worker without forcing the Agg backend (ezdxf_backend.py:1420-1444). On a host whose default MPL backend is GUI (Qt/Tk) this can hard-crash the process (SIGSEGV) — this is the real cause of the pytest warning "Starting a Matplotlib GUI outside of the main thread will likely fail." drawing_finalize calls view_screenshot, so finalize can take down the server. Fix: render via matplotlib.figure.Figure + FigureCanvasAgg (no pyplot global state), or matplotlib.use("Agg") before the pyplot import. Headless-testable.

  • I15 ★ — drawing_finalize (server.py:2333) runs only DrawingValidator().run() and never backend.drawing_critique / run_critique. The 6 premium critique focuses (iso128, layer_color, untrimmed_corner, duplicate_entities, construction_left, dim_overlap) are not part of the finalize gate — a drawing passes finalize with untrimmed corners, non-ISO-128 lineweights, leftover construction scaffolding, duplicate lines. The product's entire differentiator ("critique-then-finalize", CLAUDE.md) is advisory, not enforced. Fix: run run_critique(focus=None) inside drawing_finalize, merge into the report, gate (or warn) on critique errors. This is the moat.

  • NEW-AUTH-1 ★ — _validate_http_bind(host) is called only in the if __name__ == "__main__" block (server.py:2940). The documented launch command fastmcp run server.py:mcp --transport http --host 0.0.0.0 imports the module and never runs __main__, so the remote-bind guards (refuse remote bind without ALLOW_REMOTE_HTTP, refuse without MCP_AUTH_TOKEN) never fire under the recommended launch. With the token unset, the server binds to 0.0.0.0 anonymously with nothing to stop it. Fix: enforce the bind check at import time / in the FastMCP lifespan, not only in __main__.

  • NEW-gear-roottooth + R21 ★ — generate_involute_flank (gear.py:40-50) prepends (root_r, 0) while the involute starts at (base_r, 0) — both on y=0, so for root_r < base_r the root→base join is a straight radial segment, not a trochoid (R21). Worse, for root_r ≥ base_r (teeth ≥ ~42 @20° PA, within the teeth≥6 API limit) no root point is added and the flank starts at base_r inside the root circle → the tooth profile dips below the root arc and the closed outline self-overlaps (z=50 reproduced: outline points inside root_r). Fix: clamp the flank start to max(base_r, root_r); skip/relocate the root arc when root_r ≥ base_r; ideally a true trochoid below the base circle.

MED

  • R4 ★ — dim_overlap critique is a guaranteed no-op: _check_dim_overlap (critique.py:199-201) reads text_position/defpoint/insertion_point which no backend populates for dimensions; it also filters entity_list(layer_filter="DIM") (critique.py:192) which misses the iso13567 dim layer. Fix: populate a dim reference point in both extractors (or fall back to bbox center), and resolve the dim layer from the active set.

  • N4 — dimension_auto hardcodes layer="DIM" (base.py:948/962/970/977); construction ops hardcode "CONSTRUCTION" (base.py:869/880); _check_construction_left also hardcodes "CONSTRUCTION" (critique.py:99). For the iso13567 set (dim layer M-DIMEN-T-N, construction M-CONST-E-N) dims land on the wrong layer and leftover scaffold is never caught. Fix: resolve dim/construction layer from the active LAYER_SET_REGISTRY keyed on PlanSpec.

  • R13-COM / NEW-offset-1 ★ — ComBackend.entity_offset (com_backend.py:860-874) accepts side_x/side_y but never reads them (ent.Offset(distance) only) → COM offsets to a backend-decided side while ezdxf honors the hint. Same tool, opposite results. Also: only offset_ent[0] is kept; extra entities Offset returns leak. Fix: honor side in COM.

  • N2 (tool) — standalone drawing_save_as MCP tool defaults fmt='dwg' and doesn't derive from the extension, so drawing_save_as('part.dxf') raises on ezdxf / writes DWG bytes into a .dxf name on COM. Fix: derive fmt from the path extension in the tool too (mirror finalize).

  • R5 — COM executor calls CoInitialize() but CoUninitialize appears nowhere; on every SendCommand timeout the stuck STA thread + its COM apartment leak (a fresh executor is built), and disconnect() uses the same wait=False. Long live-AutoCAD sessions exhaust resources. Fix: worker-side try/finally CoUninitialize; bound abandoned executors.

  • R16 — COM transaction_begin/commit/rollback set _transaction_active after the await with no try/finally (com_backend.py:1398/1407/1417) → stale flag on error; rollback's raw _UNDO B (:1414) bypasses the CMDACTIVE guard. Fix: try/finally; route via _safe_send_command.

  • R23 / N6 — system_run_lisp (com_backend.py:1483-1488) sends raw LISP with no CMDACTIVE guard (deadlock on a prompting form) and captures the void SendCommand return so it always reports result:"nil". Fix: route through _safe_send_command; capture via (setvar "USERS1" …) + GetVariable, or drop the bogus result.

  • N5 — EntityInfo.properties shape diverges: ezdxf never sets bounding_box (COM always does); ezdxf TEXT sets rotation / MTEXT char_height, COM sets neither. Clients break on one backend. Fix: pick one contract; enforce both ways.

  • N3 — entity_select_smart length_range rejects all ARCs (no extractor sets ARC length). Fix: populate ARC length = radius·|Δangle| or compute on the fly.

  • NEW-dimauto-baseline-rotation — dimension_auto baseline mode (base.py:952-963) offsets perpendicular to a rotated baseline but omits rotation= → dim lines render horizontal.

  • NEW-dimauto-ordinate-misnomer — dimension_auto "ordinate" (base.py:965-979) emits two linear dims at a constant offset → overlap; not real ordinate dims.

  • NEW-com-chamfer-return — entity_chamfer (com_backend.py:1609-1629) hunts for a new AcDbLine that CHAMFER of two distinct lines never creates → almost always returns the wrong entity.

  • NEW-com-set-variable-coercion — system_set_variable (com_backend.py:1453-1458) forwards the raw value to SetVariable with no int/float coercion → type mismatch for numeric sysvars.

  • NEW-section-dupbore — gear_draw_section_aa + keyway draws the two bore lines twice → duplicate_entities critique fires and blocks the premium workflow.

  • NEW-validator-keyway — validator step-4 keyway heuristic requires a closed polyline, but draw_keyed_bore emits closed=False and the section keyway is 4 lines → permanent false-negative (keyway_section_unverified) even when a keyway exists.

  • NEW-gear-baseconstr — the gear base circle is placed on CONSTRUCTION and is silently deleted by the mandated construction_clear() before finalize → the returned handle dangles.

  • R15 — system_about tool_groups static dict omits ~22 registered tools (all engineering + premium + corner-ops + drawing_close) and misfiles entity_delete_many. Fix: derive groups from tags dynamically.

  • NEW-test-com-1 — normalize_lineweight table never tests the ambiguous bare ints 1/2 (normalize_lineweight(2)==200, i.e. 2.00 mm not 2 hundredths) — the exact P0 truncation hazard.

  • NEW-test-com-2 — COM transaction_begin/commit/rollback + the system_run_command CMDACTIVE busy-guard are entirely untested (R6 residual).

  • NEW-offset-attrs (ezdxf) — entity_offset copies only layer, dropping source color/linetype, so offsetting a CENTER/colored line yields a Continuous ByLayer line (unlike copy/mirror).

LOW

  • R22 — single _undo_stack shared by undo and transactions (no txn-id isolation); drawing_undo can eat an open transaction's snapshot; reload leaves _current_layer stale; system_status reports len(_undo_stack) as transaction_depth.
  • R24 — _apply_attrs (ezdxf_backend.py:501-502) assigns linetype without _ensure_linetype_loaded → entity_create_line(linetype="CENTER") silently renders Continuous.
  • R26 — ezdxf view_zoom_window no-op returns ok:True; view_zoom_and_screenshot ignores the window region (renders the whole modelspace).
  • R28 — COM screenshot HWND via title-substring EnumWindows instead of _acad_app().HWND.
  • R20 — _registered_tool_count reaches FastMCP private attrs with a silent -1 fallback while a public list API exists.
  • R18-residual — per-item bootstrap failures swallowed (only total exception → degraded).
  • N7 — point_from_snap ARC end/mid subscript p["start_angle"] → opaque KeyError.
  • N8 — _plan_spec written, never read; the "replayed by drawing_critique" contract is unmet.
  • NEW-base-1 — point_tangent leaks raw math domain error ValueError for from-points in [r-1e-9, r) (needs an acos-arg clamp).
  • NEW-snap-quad-arc / NEW-snap-near-arc — point_from_snap quad/near on an ARC return full-circle points that may not lie on the arc — defeats the no-guessing guarantee.
  • NEW-mtext-1 — entity_create_mtext ignores caller rotation and never reports it.
  • NEW-undo-1 — drawing_undo / transaction_begin mutate _undo_stack outside self._lock.
  • NEW-constclear-1 — construction_clear / _ensure_layer (base.py) swallow all per-entity exceptions (silent-pass introduced by the meta-tool refactor).
  • NEW-AUTH-2 — no test asserts mcp.auth is _auth or reject-on-bad-token (only construction).
  • NEW-batch-1 — entity_batch_create docstring promises a rectangle type create_map lacks, and pops type off the caller's dict.
  • NEW-test-com-3/4/5/6 — test looseness (vacuous polling assert, single-ESC vs 3, wrong boundary docstring).

SMELL

  • S5 — two layer systems (_LAYER_TEMPLATES vs LAYER_SET_REGISTRY); mechanical HIDDEN/CENTER ACI colors (1/3) swapped vs ENGINEERING_LAYERS (3/1) → template_apply_layers('mechanical') fails the layer_color critique. (The "violates ISO-128" sub-claim is refuted — template weights all pass.)
  • S3 — BlockInfo.description always empty.
  • R27 — validator step-5 SPUR/HELICAL substring match over all TEXT-layer text (no word boundary).
  • NEW-base-2 — point_intersection treats LINE as infinite line; undocumented.
  • NEW-base-3 — COM path of point_* untested.
  • NEW-claudemd-1 / doc drift — CLAUDE.md/README still list the removed CommandResult dataclass.
  • NEW-titleblock-titlex — long title x-position can overflow left of the block.

C. Fix plan (dependency-ordered)

Sprint 2 — Make the gate real + stop the crashes (CI-verifiable, headless) — ✅ DONE

Goal: the validation moat must actually run and must not crash the server. Implemented on branch fix/sprint2-gate-correctness (tests 233 → 242, ruff clean). All six items below landed + locked by tests/test_sprint2_fixes.py. See degisilik.md.

  1. NEW-screenshot-1 — force Agg rendering (Figure+FigureCanvasAgg). (S, no deps)
  2. I15 — wire run_critique(focus=None) into drawing_finalize; merge into the report; gate/warn. (M, depends on #3 to be honest)
  3. R4 + N4 — populate a dim reference point + resolve dim/construction layer from the active set; fixes both the no-op and the iso13567 mislayer. (M)
  4. NEW-AUTH-1 — enforce the remote-bind guard at import/lifespan, not only __main__. (S)
  5. NEW-gear-roottooth + R21 — clamp flank start to max(base_r,root_r), fix root-arc stitch. (M)
  6. NEW-section-dupbore + NEW-validator-keyway + NEW-gear-baseconstr — unblock the gear/keyway premium workflow end-to-end. (M)

Sprint 3 — Cross-backend parity + COM robustness (some need live AutoCAD) — ✅ DONE

Implemented on fix/sprint2-gate-correctness (tests 242 → 266). COM items code-read + mocked-COM tests (tests/test_com_sprint3.py); ezdxf/shared in tests/test_sprint3_fixes.py; R8 in tests/test_security.py. 7. R13-COM offset side ✅ (+ NEW-com-offset-orphan); N2 save_as tool fmt ✅; N5 properties parity ✅; N3 ARC length ✅. 8. R5 COM CoUninitialize/leak ✅; R16 txn try/finally + UNDO via _safe_send_command ✅; R23/N6 lisp guard + value capture ✅. (live-verify pending on real AutoCAD) 9. NEW-com-set-variable-coercion ✅; NEW-dimauto-baseline/ordinate ✅; NEW-com-chamfer-return → REFUTED on review (CHAMFER does create the bevel AcDbLine; code is correct). 10. NEW-test-com-1/2 ✅ + adversarial sanitizer tests (R8) ✅.

Sprint 4 — Hygiene + lower-severity correctness — ✅ DONE (R22 deferred)

Implemented via a 5-agent parallel workflow on fix/sprint2-gate-correctness (tests 266 → 318). Done: R24, R26, R28, R20, R15, R27, N7, N8, NEW-base-1, NEW-snap-quad-arc, NEW-snap-near-arc, NEW-undo-1, NEW-mtext-1, S3, doc drift (CLAUDE.md CommandResult + counts). New tests: tests/test_sprint4_{ezdxf,base,server,validator,com}.py. Deferred — R22 (undo/transaction stack isolation): the roadmap makes it the prerequisite for the I1 refiner's rollback cycle, so it lands with Phase A (1.2), not as standalone hygiene. Not pursued (low value / refuted): N8 implemented as docstring-fix + get_plan_spec() accessor; S5 (parallel layer systems) and NEW-constclear-1 / NEW-batch-1 left as documented SMELL.


D. Top 5 next actions

  1. NEW-screenshot-1 — Agg backend in view_screenshot/drawing_export_pdf (prevents SIGSEGV; screenshots feed the visual signal).
  2. I15 — run the premium critique inside drawing_finalize (the moat — currently advisory only).
  3. R4 + N4 — make dim_overlap fire and resolve dim/construction layers per active set (so the gate doesn't lie).
  4. NEW-AUTH-1 — enforce the remote-bind guard on the documented fastmcp run path (fail-open security gap).
  5. NEW-gear-roottooth + R21 — fix the self-overlapping tooth outline for teeth ≥ ~42.

E. Risks / notes

  • Live-AutoCAD only: R5, R16, R23/N6, R28, NEW-com-chamfer/set-variable can't be CI-verified here — gate them behind mocked-COM tests + manual live check.
  • Cross-backend parity is the dominant theme of the open work (Sprint-1 fixes were mostly one-backend). Every new behavior should land on both backends or explicitly document the gap.
  • Security residual: the audit:security-hardening finder was blocked by a provider cyber-safety filter, so the deny-list bypass analysis (R7/R8) is not complete. The sanitizer is wired (server.py:2055/2072) and test_security.py exists, but adversarial bypass coverage remains an open defensive item — handle in Sprint 3 #10.