nc_program_manager/.paul/codebase/CONCERNS.md

Codebase Concerns

Analysis Date: 2026-05-18

Tech Debt

Blocking async calls at CLI boundary:

• Issue: 56+ .GetAwaiter().GetResult() calls block async operations synchronously
• Files: Program.cs (lines 90, 101, 122, 142, 166, 200, 221, 240, 262), machine classes
• Why: Main() was not async when initially written; .NET 4.7.2 supports async Main via C# 7.1
• Impact: Thread pool starvation risk, deadlock risk in certain SynchronizationContext scenarios, harder debugging
• Fix: Refactor Program.Main to static async Task<int> Main(string[] args), propagate await throughout

Silent exception handlers (catch without logging):

• Issue: try { ... } catch { } swallows exceptions with no trace
• Files: Cnc/Fanuc/FanucMachine.cs (disconnect cleanup), Cnc/Siemens/SiemensMachine.cs, Cnc/Heidenhain/HeidenhainMachine.cs, Cnc/Heidenhain/Lsv2Client.cs
• Why: Cleanup paths (disconnect, logout) should not propagate to caller; silence was intentional
• Impact: Resource leaks masked, production issues undiagnosable, disconnect failures invisible
• Fix: Log at minimum to debug/trace: catch (Exception ex) { /* log ex */ }. Add logging framework.

No structured logging framework:

• Issue: Entire codebase uses Console.WriteLine (98+ calls in Program.cs alone)
• Files: Program.cs, all machine implementations
• Why: MVP simplicity; tool was always a CLI
• Impact: No log levels, no timestamps, no file logging, production issues undiagnosable
• Fix: Add Serilog or NLog. Replace Console.WriteLine with _logger.Information(). Add request/response tracing to FOCAS calls.

Large monolithic FanucMachine:

• Issue: Cnc/Fanuc/FanucMachine.cs is ~740 lines with complex state management
• Files: Cnc/Fanuc/FanucMachine.cs
• Why: FOCAS protocol has many edge cases; chunked transfer and offset handling grew organically
• Impact: Hard to test individual branches; buffer handling edge cases unclear
• Fix: Extract IFocasTransferHandler for chunk upload/download logic; separate offset data reader

string.Format() over $"" interpolation:

• Issue: Old string formatting style throughout validator and some machine code
• Files: Cnc/NcProgramValidator.cs, Program.cs
• Why: Legacy code predates C# 6 adoption on this project
• Impact: Cosmetic; harder to read format strings
• Fix: Opportunistic migration to $"" during edits

Known Issues / Incomplete Implementations

Heidenhain ReadActiveProgramAsync / SelectMainProgram not implemented:

• Symptoms: Returns CncResult.Fail("Not supported via basic LSV2")
• Files: Cnc/Heidenhain/HeidenhainMachine.cs
• Root cause: LSV2 protocol complexity; feature not required for initial release
• Workaround: Users specify program path explicitly

Siemens ReadActiveProgramAsync / SelectMainProgram not implemented:

• Symptoms: Returns stub failure
• Files: Cnc/Siemens/SiemensMachine.cs
• Root cause: FTP protocol doesn't expose active program info
• Workaround: Users specify program path explicitly

MachineSnapshot returns stub data for Heidenhain/Siemens/Mitsubishi:

• Files: Cnc/Heidenhain/HeidenhainMachine.cs, Cnc/Siemens/SiemensMachine.cs, Cnc/Mitsubishi/MitsubishiMachine.cs
• Issue: All snapshot fields return Unknown/empty; no real machine state read
• Impact: No diagnostic info available for non-Fanuc machines

Security Considerations

RSA public key hardcoded in binary:

• Risk: Key embedded as const string in binary; cannot be rotated without recompile and redeploy
• Files: Licensing/LicenseValidator.cs
• Current mitigation: Asymmetric RSA — private key is vendor-only; hardcoded public key is acceptable for this use case
• Recommendation: Document key rotation procedure; consider adding license version/expiration metadata for future revocation

MachineFingerprint null dereference on unusual hardware:

• Risk: mo.Properties["processorID"].Value.ToString() throws NullReferenceException if WMI property missing
• Files: Licensing/MachineFingerprint.cs
• Current mitigation: None
• Fix: mo.Properties["processorID"]?.Value?.ToString() ?? "unknown"

Obsolete crypto APIs:

• Risk: RSACryptoServiceProvider and SHA256CryptoServiceProvider are obsolete in .NET 6+
• Files: Licensing/LicenseValidator.cs
• Current mitigation: .NET 4.7.2 target — obsolete warning only, not broken
• Fix when upgrading: Use RSA.Create() + VerifyData(data, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)

SHA256 provider not explicitly disposed:

• Risk: Minor resource leak in license validation path
• Files: Licensing/LicenseValidator.cs
• Fix: Wrap in using (var sha = new SHA256CryptoServiceProvider()) { ... }

Platform Constraints

Windows-only (hard constraint):

• FOCAS1 DLL (Fwlib32.dll) is 32-bit Windows native — no Linux/macOS support
• WMI (System.Management) Windows-only — licensing system non-functional on Linux
• Files: Cnc/Fanuc/libs/, Licensing/MachineFingerprint.cs, fwlib32.cs
• Impact: Cannot run on Linux/macOS; CI on Linux can only test non-FOCAS paths
• Action if cross-platform needed: Abstract fingerprinting behind IMachineFingerprint; already done — add Linux implementation using /proc/cpuinfo or dmidecode

FOCAS DLL version sensitivity:

• Risk: FOCAS API changes between Fanuc control versions; FanucDialectDetector auto-detects but may fail on unknown versions
• Files: Cnc/Fanuc/FanucDialectDetector.cs, Cnc/Fanuc/FanucMachine.cs
• Mitigation: Dialect abstraction layer (IFocasDialect) isolates version differences

Performance Bottlenecks

In-memory accumulation for large NC programs:

• Issue: UploadProgramByIdentifier appends to StringBuilder for entire program before writing
• Files: Cnc/Fanuc/FanucMachine.cs
• Impact: For programs >10MB, high memory allocation; streaming to file would be more efficient
• Fix: Write chunks directly to FileStream instead of accumulating in memory

Polling with 100ms busy-wait during transfer:

• Issue: ct.WaitHandle.WaitOne(100) loops during buffer waits
• Files: Cnc/Fanuc/FanucMachine.cs
• Impact: Wastes thread-pool slot; CPU cycles during transfer
• Fix: Use await Task.Delay(100, ct) instead of blocking WaitOne

Network connection timeout via .Wait() with boolean return:

• Issue: _tcp.ConnectAsync().Wait(timeout) — if task never completes, does not abort it
• Files: Cnc/Heidenhain/Lsv2Client.cs
• Fix: await Task.WhenAny(connectTask, Task.Delay(timeout, ct)) + cancellation

Fragile Areas

FOCAS dialect detection (FanucDialectDetector):

• Files: Cnc/Fanuc/FanucDialectDetector.cs
• Why fragile: Relies on probing specific FOCAS API calls to detect version; untested against full range of Fanuc control generations
• Safe modification: Always test against physical hardware after changes; log detected dialect version

License validation (no recovery path):

• Files: Licensing/LicenseValidator.cs, Licensing/MachineFingerprint.cs
• Why fragile: Any WMI failure or file read error produces a hard exit with no diagnostic for user
• Safe modification: Add exception handling with user-facing error message; distinguish "no license file" from "invalid license" from "fingerprint failure"

Integration tests with hardcoded IPs:

• Files: NcProgramManager.Tests/Integration/SiemensMachineIntegrationTests.cs, HeidenhainMachineIntegrationTests.cs, MitsubishiMachineIntegrationTests.cs
• Why fragile: Tests will fail in CI without physical hardware at specific IPs
• Fix: Make IPs configurable via environment variables; skip integration tests in CI by default

Missing / Not Covered

• No CLI-level tests (no test for Program.cs orchestration or exit code behavior)
• No tests for MachineFingerprint (WMI, no mock tests)
• No tests for LicenseFile parsing edge cases (corrupt Base64, empty file)
• No structured logging or diagnostics framework
• No error code documentation in code (only in README.md)
• No architecture documentation (FOCAS dialect selection logic undocumented in code)

---

Overall Assessment: Architecture is solid — clean machine abstraction, well-structured interfaces, good error propagation via CncResult<T>. Main concerns are operational (logging, blocking async, empty catch blocks) rather than structural. Platform constraints (Windows, FOCAS DLL) are intentional for the target environment.

---

Concerns analysis: 2026-05-18 Update as issues are resolved or discovered