nc_program_manager/.paul/ISSUES.md

# Project Issues Log

Enhancements and known issues discovered during execution and codebase analysis.

## Open Issues

### ~~ISS-001: MachineFingerprint null dereference on unusual hardware~~ ✅ CLOSED

- **Closed:** 2026-05-18 (Phase 13, commit `4cd7715`)
- **Fix applied:** `?.Value?.ToString() ?? string.Empty` on both WMI reads in `Licensing/MachineFingerprint.cs`

---

### ISS-002: Blocking async calls at CLI boundary

- **Discovered:** Codebase analysis (2026-05-18)
- **Type:** Refactoring / Reliability
- **Description:** 56+ `.GetAwaiter().GetResult()` calls in `Program.cs` and machine classes block the thread pool. .NET 4.7.2 supports `async Task<int> Main`. Risk of deadlock in certain SynchronizationContext scenarios.
- **Files:** `Program.cs` (lines 90, 101, 122, 142, 166, 200, 221, 240, 262)
- **Fix:** Refactor `Main` to `static async Task<int> Main(string[] args)`, propagate `await` throughout
- **Impact:** Medium — currently works, but fragile; blocks async benefits
- **Effort:** Medium (1-2 hours)
- **Suggested phase:** Future refactor phase

---

### ~~ISS-003: Silent exception handlers swallow failures~~ ✅ CLOSED

- **Closed:** 2026-05-18 (Phase 15)
- **Fix applied:** Event handler catches (`SetState`) → `_log.Warn(ex, "StateChanged handler threw")`; cleanup/Dispose catches annotated with `/* cleanup — intentionally swallowed */`

---

### ~~ISS-004: No structured logging framework~~ ✅ CLOSED

- **Closed:** 2026-05-18 (Phase 15)
- **Fix applied:** NLog 5.2.8 wired into all 5 production classes; rolling file target (daily, 7-day archive) + console target (Warn+); `_log.Error` in Program.cs error catch blocks

---

### ~~ISS-005: SHA256 crypto provider not disposed~~ ✅ CLOSED

- **Closed:** 2026-05-18 (Phase 13, commit `4cd7715`)
- **Fix applied:** `SHA256CryptoServiceProvider` wrapped in `using` block in `Licensing/LicenseValidator.cs`

---

### ~~ISS-006: LSV2 connect timeout doesn't abort hung task~~ ✅ CLOSED

- **Closed:** 2026-05-18 (Phase 13, commit `4cd7715`)
- **Fix applied:** `_tcp.Close()` called in timeout branch of `Connect()` in `Cnc/Heidenhain/Lsv2Client.cs` — aborts pending socket task

---

### ~~ISS-007: Integration tests use hardcoded IPs~~ ✅ CLOSED

- **Closed:** 2026-05-18 (Phase 14)
- **Fix applied:** `HardwareTestHelper.GetIpOrSkip(envVarName)` reads `HEIDENHAIN_IP` / `SIEMENS_IP` / `MITSUBISHI_IP`; `Assert.Ignore` fires when env var not set. Hardware tests `[Category("Hardware")]` skip in standard CI; stub-based tests unaffected.

---

### ISS-008: MachineFingerprint — full license flow unverified on real hardware

- **Discovered:** STATE.md deferred issues (ongoing)
- **Partial fix:** 2026-05-18 (Phase 14) — `LicenseFile` edge-case tests added; `Validate_EmptyFingerprint_ReturnsFalse` confirms validator is safe against empty input
- **Remaining:** Full end-to-end (WMI fingerprint → sign → deploy → validate) requires real Windows machine run with `HEIDENHAIN_IP` etc. set
- **Type:** Testing
- **Files:** `Licensing/MachineFingerprint.cs`, `Licensing/LicenseFile.cs`
- **Impact:** High — license flow unverified on hardware before customer delivery
- **Effort:** Quick — scaffold is in place; just needs real hardware

---

### ISS-009: Heidenhain ReadActiveProgramAsync / SelectMainProgram not implemented

- **Discovered:** STATE.md deferred issues (2026-05-13)
- **Type:** Feature / Incomplete Implementation
- **Description:** `HeidenhainMachine.ReadActiveProgramAsync()` and `SelectMainProgramAsync()` return hard-coded `CncResult.Fail("Not supported via basic LSV2")`. LSV2 may support these via additional commands not yet implemented.
- **Files:** `Cnc/Heidenhain/HeidenhainMachine.cs`
- **Impact:** Low — current use case requires explicit program path; feature unused
- **Effort:** Substantial (requires LSV2 spec research + hardware testing)
- **Suggested phase:** Heidenhain advanced features (future)

---

### ISS-010: Siemens ReadActiveProgramAsync / SelectMainProgram not implemented

- **Discovered:** STATE.md deferred issues (2026-05-13)
- **Type:** Feature / Incomplete Implementation
- **Description:** Same as ISS-009 for Siemens. FTP protocol may not expose active program info natively.
- **Files:** `Cnc/Siemens/SiemensMachine.cs`
- **Impact:** Low — current use case requires explicit program path
- **Effort:** Substantial (requires Sinumerik FTP extension research)
- **Suggested phase:** Siemens advanced features (future)

---

### ISS-011: Obsolete crypto APIs (RSACryptoServiceProvider, SHA256CryptoServiceProvider)

- **Discovered:** Codebase analysis (2026-05-18)
- **Type:** Technical Debt
- **Description:** Both classes marked obsolete in .NET 6+. Currently safe on .NET 4.7.2 (only warnings), but blocks future framework upgrade path.
- **Files:** `Licensing/LicenseValidator.cs`
- **Fix (when upgrading):** `RSA.Create()` + `rsa.VerifyData(data, sig, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)` — eliminates both obsolete types
- **Impact:** Low — no runtime impact on .NET 4.7.2
- **Effort:** Quick (when .NET version is upgraded)
- **Suggested phase:** Future .NET upgrade phase

---

### ISS-012: Fagor `ReadActiveProgramAsync` / `SelectMainProgramAsync` not implemented

- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Feature / Incomplete Implementation
- **Description:** `FagorMachine.ReadActiveProgramAsync()` e `SelectMainProgramAsync()` ritornano `CncResult.Fail("Not supported via FTP")`. Analogo a ISS-009 (Heidenhain) e ISS-010 (Siemens). Fagor espone stato runtime via DNC/OPC-UA non FTP.
- **Files:** `Cnc/Fagor/FagorMachine.cs`
- **Impact:** Low — caso d'uso corrente richiede path esplicito
- **Effort:** Substantial (richiede protocollo DNC/OPC-UA Fagor)
- **Suggested phase:** Future Fagor advanced features

---

### ISS-013: `Program.Invia` accoppia `FanucProgram` parsing a tutti i produttori

- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Bug / Refactoring
- **Description:** `Program.Invia` crea `new FanucProgram(File.ReadAllText(...))` e chiama `GetProgramTitle()` con regex `O####` o `\d{4}` per tutti i produttori. Per Siemens/Mitsubishi/Fagor con programmi non in formato Fanuc, lancia `InvalidOperationException`. Fagor incrementa esposizione del bug.
- **Files:** `Program.cs` (linee 188-202), `FanucProgram.cs`
- **Fix:** Per non-Fanuc usare `Path.GetFileNameWithoutExtension(inputArgs.pathLocaleProgramma)` come `cncProg.Name`. Bypassare parsing Fanuc.
- **Impact:** Medium — CLI `Invia` rotta per tutti i non-Fanuc (Siemens/Mitsubishi/Fagor)
- **Effort:** Medium (1-2h) — refactor Program.Invia con switch su manufacturer
- **Suggested phase:** Future CLI cleanup phase

---

### ISS-014: `ResolvePath` null-folder NRE shared pattern (Siemens/Mitsubishi/Fagor)

- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Bug / Defensive coding
- **Description:** `ResolvePath(folder, name)` in tutti e 3 le macchine FTP dereferenzia `folder` senza null-guard nel ramo `IsNullOrEmpty(name)`. Caller che passa `path = null` (es. `ReadProgramAsync(null)`) provoca NullReferenceException.
- **Files:** `Cnc/Siemens/SiemensMachine.cs`, `Cnc/Mitsubishi/MitsubishiMachine.cs`, `Cnc/Fagor/FagorMachine.cs`
- **Fix:** `if (folder == null) folder = _config.ProgramDirectory;` all'inizio del metodo. Pattern condiviso → considerare helper unitario.
- **Impact:** Low — null path raro in pratica, ma viola contratto API
- **Effort:** Quick (10 min) — refactor uniforme

---

### ISS-015: Encoding source-file rischio `ñ` su build Windows non-UTF8

- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Build / Encoding
- **Description:** File sorgenti UTF-8 senza BOM. `csc.exe` su Windows con codepage 1252 (Italian) può decodificare letterali non-ASCII come Mojibake. Risolto in v0.6 (Phase 16-02) per `ñ` in `NcProgramValidator.cs` usando `ñ`. Altri file (`Program.cs` con `è`, `à` in stringhe italiane) ancora potenzialmente a rischio. Funziona se build box codepage compatibile, ma fragile.
- **Files:** `Program.cs`, `Cnc/NcProgramValidator.cs`, file `.cs` con letterali italiani
- **Fix:** (a) Aggiungere UTF-8 BOM a tutti i sorgenti, OR (b) sostituire tutti letterali non-ASCII con escape `\uXXXX`, OR (c) aggiungere `/codepage:65001` ai compile args MSBuild.
- **Impact:** Medium — silent corruption se build box ha codepage diversa
- **Effort:** Medium (audit completo file + correzione)
- **Suggested phase:** Future build hardening phase

---

### ISS-016: `WriteProgramAsync` validator riceve `path` se `program.Name` null (Siemens/Mitsubishi/Fagor)

- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** API contract / Validation
- **Description:** `_validator.Validate(program.Name ?? path, program.Content)` — se `program.Name` null, validator valida l'intero path FTP come fosse il nome programma. Errori validatore confusi.
- **Files:** `Cnc/Siemens/SiemensMachine.cs`, `Cnc/Mitsubishi/MitsubishiMachine.cs`, `Cnc/Fagor/FagorMachine.cs`
- **Fix:** Fail-fast con `CncError("WriteProgram", "Program.Name is required")` quando `program.Name == null`. Documentare in `CncProgram.cs`.
- **Impact:** Low — caller di solito imposta `Name`
- **Effort:** Quick

---

### ISS-017: `FagorFtpClient.Ping` swallow exception senza log

- **Discovered:** Code review v0.6 (2026-05-25)
- **Type:** Diagnostics
- **Description:** `Ping()` catch generico ritorna `false` senza log. Stesso pattern in Siemens/Mitsubishi. Diagnostica ridotta su problemi di connessione (timeout vs credenziali vs path mancante).
- **Files:** `Cnc/Siemens/SiemensFtpClient.cs`, `Cnc/Mitsubishi/MitsubishiFtpClient.cs`, `Cnc/Fagor/FagorFtpClient.cs`
- **Fix:** Aggiungere campo `_log` + `_log.Debug(ex, "FTP ping failed")` nel catch. Mantenere return false. Pattern condiviso.
- **Impact:** Low — diagnostica
- **Effort:** Quick

---

### ~~ISS-018: `Standardize()` `\n`-prefix breaks `NcProgramValidator` first-line check~~ ✅ CLOSED

- **Discovered:** Field test docker FTP (2026-06-02)
- **Closed:** 2026-06-02
- **Type:** Bug (CRITICAL)
- **Description:** `FanucProgram.Standardize()` prepends `"\n"` to `Program` (`FanucProgram.cs:147-153`). `NcProgramValidator` first-line check reads `lines[0]` and expects `^O\d{4}` (`NcProgramValidator.cs:40-46`). After standardize, `lines[0] == ""` → check always fails `"First line must be O#### program number"`. **Every Fanuc/Mitsubishi upload rejected**, including valid `O0001(TEST HENESIS)`. Confirmed via field test: all 5 T03 samples + T01 + T07 INVIO returned `-204`.
- **Files:** `Cnc/NcProgramValidator.cs`, `FanucProgram.cs`
- **Fix applied:** validator first-line check now scans first **non-blank** line instead of `lines[0]`, tolerating the leading `\n` that `Standardize()` injects. Upload format unchanged (FOCAS tape format preserved).
- **Impact:** Critical — no valid program uploadable for Fanuc/Mitsubishi
- **Effort:** Quick

---

### ~~ISS-019: `-pathprogramma` truncated at first space (unquoted `.bat` args)~~ ✅ CLOSED

- **Discovered:** Field test docker FTP (2026-06-02)
- **Closed:** 2026-06-02
- **Type:** Bug (CRITICAL)
- **Description:** Field-test `.bat` files pass `-pathprogramma=%~dp0samples\...` and `%RESULTS%\...` **unquoted**. When the install path contains spaces (`...\visual studio prove\...`), cmd splits the argument into multiple argv tokens. `ArgParser.cs:73` `Find` matches only the first token → `Substring(15)` yields a truncated path (e.g. `C:\Users\trent\Desktop\davide\visual`). Effect: SCARICA writes the download to the wrong file (still exit 0, masking failure); INVIA later reads that stray file → garbage. Explains the observed T02→T01 interaction.
- **Files:** `field-test/t01_invia_valido.bat`, `t02_scarica.bat`, `t03_validatore.bat`, `t04_errori_connessione.bat`, `t05_offset_non_supportati.bat`, `t07_roundtrip.bat`
- **Fix applied:** all `-pathprogramma=...` args quoted (`-pathprogramma="..."`) across the 6 `.bat` files. `ArgParser` itself is correct when argv is passed properly (Windows delivers a quoted spaced path as a single token).
- **Impact:** Critical — all file I/O hit wrong path on space-containing install dirs
- **Effort:** Quick

---

### ISS-020: Write/Read FTP path asymmetry — `ResolvePath` (Mitsubishi fixed; Siemens/Fagor open)

- **Discovered:** Field test docker FTP, post ISS-018/019 fix (2026-06-02)
- **Mitsubishi fix applied:** 2026-06-02 — `WriteProgramAsync` now resolves the path identically to `ReadProgramAsync`; `ResolvePath` simplified to single-arg (`path` = identifier or full path, combined with `ProgramDirectory`). Verified against Mitsubishi NC Explorer manual IB-1500904 (`PRG/USER/<Program name>`, no extension, addressed by program name/number). Unit tests updated + roundtrip regression test added. `program.Name` now used for validation only.
- **Still open (Siemens/Fagor):** same asymmetric pattern, but their `ResolvePath` ties extension handling (`.MPF` / `.nc`) to `name` — needs separate fix + entangled with ISS-013 (Fanuc-parsing coupling in `Program.Invia`). Untested (no field rig yet).
- **Default ProgramDirectory note:** real Mitsubishi path is `/PRG/USER/`; current config default is `/PRG/`. Adjust per controller (config-level, not code).
- **Type:** Bug (design) — criticità C2/C3
- **Description:** `-path` integer is resolved differently for read vs write.
  - **Read** `ResolvePath("1", null)` → `"1"` has no `/` → `ProgramDirectory + "1"` = `/PRG/1` (correct, file exists).
  - **Write** `ResolvePath("1", program.Name="O0001")` → `folder="1"` treated as directory → `"1/" + "O0001"` = `1/O0001` (relative dir `1` does not exist → STOR fails with `WebException` surfaced as "Impossibile effettuare la connessione al server remoto").
  Confirmed against docker FTP: `STOR 1/O0001` → curl exit 9; `STOR /PRG/1` → exit 0; no `1/` dir created (app does not create dirs).
- **Effect:** INVIO of any valid program fails (`-204`); roundtrip impossible — write target ≠ read target. Was previously masked by ISS-018 (validator blocked all uploads) and ISS-019 (truncated path).
- **Files:** `Cnc/Mitsubishi/MitsubishiMachine.cs` `ResolvePath`, same pattern in `Cnc/Siemens/SiemensMachine.cs`, `Cnc/Fagor/FagorMachine.cs`
- **Open design question:** canonical Mitsubishi FTP filename — bare integer (`/PRG/1`, symmetric with read) vs ISO title (`/PRG/O0001`)? Needs real-CNC convention. ResolvePath write branch was designed assuming `path` is a *directory*; CLI passes it as a program *identifier*.
- **Impact:** High — CLI `Invia` broken for all FTP makers
- **Effort:** Medium — fix ResolvePath + decide naming convention; verify all 3 makers
- **Related:** ISS-013 (Fanuc parsing coupling)

---

### ISS-021: Mazak support — requires EIA/ISO option active on control (Mazatrol out of scope)

- **Discovered:** Intake/online research for Mazak integration (2026-06-16)
- **Type:** Enhancement constraint / known limitation — pre-implementation
- **Decision:** Mazak support will transfer **EIA/ISO G-code (plain text) over FTP only**, reusing the Mitsubishi/Siemens/Fagor `*FtpClient` pattern. Target controllers: Smooth family (SmoothG/X/Ai/EZ).
- **Hard requirement — EIA/ISO option:** modern Mazatrol controls run both Mazatrol conversational and EIA/ISO G-code, but **EIA/ISO is frequently a license-gated option** (paid activation) and is NOT guaranteed present on a given machine. If the EIA option is NOT active on the target control, this feature cannot send programs to that machine — only Mazatrol (proprietary binary) would work there, which is out of scope. **Operator must verify the EIA/ISO option is enabled on the control before use.**
- **Mazatrol CMT out of scope:** Mazatrol programs are a proprietary binary format (CMT) with no public PC-side authoring/transfer API. Supporting them would require a licensed vendor library (CIMCO/MazaCAM) or a DNC box (e.g. Mazak LAN Connect, Calmotion LANUSB-MAZ). Explicitly excluded.
- **No FOCAS equivalent:** unlike Fanuc (FOCAS `fwlib32.dll`) and Mitsubishi, Mazak exposes no native DLL/SDK for program up/download. MTConnect / SmartBox / SmartLink are monitoring-only (read), not an upload channel. Transfer is file-drop (FTP/SMB/serial) only.
- **Config variance to expose (not hardcode):**
  - FTP port: **21 vs 23** — some Smooth Ai units run FTP via IIS on port 23 because port 21 conflicts with Mazak's own boot/shutdown data-backup. Default 21, configurable.
  - Credentials + NC program directory: undocumented, site-specific (NC dir often surfaced via `MC_DIRECT MODE PROGRAMS` share or a user-created exchange folder). Configurable, no safe public default.
  - File extension: `.eia` default (configurable, like Fagor `.nc`).
- **README note pending:** add Mazak row to "Controller supportati" table + a `### Controller Mazak` section documenting the EIA-option requirement **when the feature is implemented** (not before — avoid claiming support that doesn't exist yet).
- **Validator ruleset (VERIFIED online 2026-06-16 vs Mazak Matrix EIA manual H740PB0030E + field reports):** Mazak EIA is **NOT** Fanuc-restrictive. Do NOT reuse the Fanuc/Mitsubishi restrictive gate. Verified facts:
  - Program number = `O` + **up to 8 digits** (`O[0-9]{1,8}`), and is **optional** — programs may lead with `%` (EOR) or have no O-line. A 4-digit-required `O####` check would reject valid programs.
  - `;` is the **literal EOB (End-Of-Block) code** in Mazak EIA file content — MUST be allowed, not forbidden.
  - `[` `]` `#` `*` `=` `:` are valid ISO codes (TAP9–TAP14) — allow.
  - `( )` = comment Control-Out/In — allowed; only nested parens are risky.
  - No 80-char block cap (input buffer 248×5). Drop the cap.
  - Lowercase: not in EIA punch-code table → likely insignificant; UNCERTAIN for FTP text upload — verify on real control before hard-reject, do not assume.
  - → Mazak validator must be **lenient** (optional 1–8 digit O-header, permissive char set), NOT added to `IsRestrictiveDialect()`.
- **Addressing (verified UNCERTAIN):** over FTP a program is a file on a Windows filesystem → addressed by **full filename incl. `.eia`**, not bare O-number (control maps file↔work-number internally). Verified: each program/subprogram = separate file named by number sans `O` (e.g. `O1234` content → file `1234.eia`); main calls subprograms at runtime via `M98 P1234`; control links files by name/number at execution, NOT bundled.
- **DECISION (user, 2026-06-16) — preserve filenames verbatim:** O#### naming is a Fanuc-only constraint (old-NC name reuse across same family). NOT relevant for Mazak. Therefore:
  - `MazakMachine.ResolvePath` uses the operator-supplied filename **verbatim** — no O-number derivation, no rename, **no auto-`.eia` append**. Symmetric read/write on the same literal name. `MazakConnectionConfig` has NO `ProgramExtension` field.
  - Validator for Mazak = **FULL PASSTHROUGH** — `Validate(CncManufacturer.Mazak, …)` returns `ValidationResult.Success` unconditionally. No name check, no content check (FTP sends file as-is). Mazak NOT in `IsRestrictiveDialect()`, no forbidden-char/header/length rules. Verified Mazak EIA permissiveness (`;` EOB, `[ ]`, `#` valid) makes restrictive checks wrong anyway; passthrough is the deliberate choice.
  - Multi-file jobs (main + subprograms) sent as **separate transfers**, each file keeping its own name.
- **`.eia` extension + port 21/23 + license-gating:** CONFIRMED online (Versicor Smooth Ai FTP article; Practical Machinist EIA-option threads). `.eia` vs `.EIA` case + exact NC dir path per Smooth firmware = still site-specific, settle on real machine.
- **Impact:** Medium — feature usable only where EIA option licensed; documented limitation, not fixable in code.
- **Effort:** Medium — mirrors Fagor phase (machine + FtpClient + config + **lenient** validator ruleset + factory + tests).
- **Suggested phase:** Future milestone v0.8 — Mazak Support.

---

### ISS-022: `DeleteProgramAsync` raw-path asymmetry (Mitsubishi — same class as ISS-020)

- **Discovered:** Mazak implementation review (2026-06-16) — Mazak copied the pattern, then fixed its own copy.
- **Type:** Bug (design) — path asymmetry.
- **Description:** `MitsubishiMachine.DeleteProgramAsync` calls `_ftp.DeleteFile(path)` with the RAW path, bypassing `ResolvePath`, while `ReadProgramAsync`/`WriteProgramAsync` resolve via `ResolvePath`. So `Delete("1")` targets `1` (relative) instead of `/PRG/1` — asymmetric with read/write, same defect family as ISS-020 (which fixed read/write but left Delete).
- **Files:** `Cnc/Mitsubishi/MitsubishiMachine.cs` `DeleteProgramAsync` (~line 122). Likely also Siemens/Fagor — verify.
- **Mazak status:** FIXED in new code (`MazakMachine.DeleteProgramAsync` resolves via `ResolvePath`). Mitsubishi/Siemens/Fagor still open.
- **Impact:** Medium — delete-by-identifier hits wrong FTP path; delete fails or no-ops silently. Masked until someone deletes by bare number.
- **Effort:** Low — one-line per maker + regression test.
- **Related:** ISS-020.

---

## Closed Issues

| ID | Summary | Phase | Commit |
|----|---------|-------|--------|
| ISS-001 | MachineFingerprint null dereference on unusual hardware | 13 | `4cd7715` |
| ISS-005 | SHA256 crypto provider not disposed | 13 | `4cd7715` |
| ISS-006 | LSV2 connect timeout doesn't abort hung task | 13 | `4cd7715` |
| ISS-007 | Integration tests use hardcoded IPs | 14 | `32f68c5` |
| ISS-003 | Silent exception handlers swallow failures | 15 | (Phase 15 commit) |
| ISS-004 | No structured logging framework | 15 | (Phase 15 commit) |
| ISS-018 | Standardize() \n-prefix breaks validator first-line check | field-test | (pending) |
| ISS-019 | -pathprogramma truncated at first space (unquoted .bat) | field-test | (pending) |

---

*Last updated: 2026-06-02*
*Sources: STATE.md deferred issues + CONCERNS.md codebase analysis + field-test docker FTP*