autocad-mcp-pro/docs/analysis/03-AUDIT-VERIFICATION.md

<!--
Generated by the AUDIT-AND-PLAN workflow (wf_f688392c-339), 2026-06-18.
Phase 1 (11 finders) completed; Phase 2 (adversarial verify) + Phase 3 (synthesis) hit the
session token limit, so verification + synthesis were completed inline by the main agent,
with 6 of the highest-impact findings re-confirmed by direct code reading (100% confirm rate).
Pairs with 01-DIAGNOSTICS.md (the pre-Sprint-1 baseline) and degisilik.md (Sprint-1 changelog).
-->

# 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.