diff --git a/.paul/ISSUES.md b/.paul/ISSUES.md index 59ef6ec..990ab04 100644 --- a/.paul/ISSUES.md +++ b/.paul/ISSUES.md @@ -208,6 +208,26 @@ Enhancements and known issues discovered during execution and codebase analysis. --- +### 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/`, 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) + +--- + ## Closed Issues | ID | Summary | Phase | Commit | diff --git a/Cnc/Mitsubishi/MitsubishiMachine.cs b/Cnc/Mitsubishi/MitsubishiMachine.cs index 6c2ab66..ba1fa61 100644 --- a/Cnc/Mitsubishi/MitsubishiMachine.cs +++ b/Cnc/Mitsubishi/MitsubishiMachine.cs @@ -70,7 +70,7 @@ namespace NcProgramManager.Cnc.Mitsubishi return RunGuardedAsync(() => { EnsureConnected(); - string ncPath = ResolvePath(path, null); + string ncPath = ResolvePath(path); try { return _ftp.DownloadFile(ncPath); } catch (Exception ex) { @@ -99,7 +99,12 @@ namespace NcProgramManager.Cnc.Mitsubishi _errors.Add(new CncError(0, "Validator", msg)); return false; } - string ncPath = ResolvePath(path, program.Name); + // ISS-020: resolve the write path the same way ReadProgramAsync does. + // `path` is the program identifier (e.g. "1" -> /PRG/1) or a full FTP + // path, NOT a directory to combine with program.Name. On Mitsubishi the + // file in PRG/USER is addressed by its program name/number; program.Name + // (the ISO O-number) is used for validation only, not the filename. + string ncPath = ResolvePath(path); try { _ftp.UploadFile(ncPath, program.Content); return true; } catch (Exception ex) { @@ -199,15 +204,16 @@ namespace NcProgramManager.Cnc.Mitsubishi } } - /// Build full FTP path. Mitsubishi programs have no extension. - private string ResolvePath(string folder, string name) + /// + /// Build full FTP path. Mitsubishi programs have no extension and are addressed + /// by their program name/number under . + /// A bare identifier ("1") is combined with the program directory ("/PRG/1"); + /// an explicit path containing '/' is used as-is. Read and write resolve identically. + /// + private string ResolvePath(string path) { - if (string.IsNullOrEmpty(name)) - return folder.Contains("/") ? folder : _config.ProgramDirectory + folder; - - string dir = folder ?? _config.ProgramDirectory; - if (!dir.EndsWith("/")) dir += "/"; - return dir + name; + if (string.IsNullOrEmpty(path)) return _config.ProgramDirectory; + return path.Contains("/") ? path : _config.ProgramDirectory + path; } } } diff --git a/NcProgramManager.Tests/Unit/MitsubishiMachineTests.cs b/NcProgramManager.Tests/Unit/MitsubishiMachineTests.cs index 93c1706..839dc3f 100644 --- a/NcProgramManager.Tests/Unit/MitsubishiMachineTests.cs +++ b/NcProgramManager.Tests/Unit/MitsubishiMachineTests.cs @@ -96,20 +96,38 @@ namespace NcProgramManager.Tests.Unit // WriteProgramAsync [Test] - public void WriteProgramAsync_ValidContent_CallsUploadFileNoExtension() + public void WriteProgramAsync_FullPath_CallsUploadFileNoExtension() { + // ISS-020: write resolves the path identically to read (path = program + // identifier / full path, not a directory combined with program.Name). _ftpMock.Setup(f => f.Ping()).Returns(true); _ftpMock.Setup(f => f.UploadFile("/PRG/O1234", "CONTENT")); _machine.ConnectAsync().GetAwaiter().GetResult(); var prog = new CncProgram { Name = "O1234", Content = "CONTENT" }; - var result = _machine.WriteProgramAsync("/PRG/", prog).GetAwaiter().GetResult(); + var result = _machine.WriteProgramAsync("/PRG/O1234", prog).GetAwaiter().GetResult(); Assert.That(result.Success, Is.True); _ftpMock.Verify(f => f.UploadFile("/PRG/O1234", "CONTENT"), Times.Once); _ftpMock.Verify(f => f.UploadFile(It.Is(s => s.EndsWith(".MPF")), It.IsAny()), Times.Never); } + [Test] + public void WriteProgramAsync_ShortName_ResolvesWithDirectory_SymmetricWithRead() + { + // ISS-020 regression: -path "1" must write to ProgramDirectory + "1" + // (the same location ReadProgramAsync("1") reads), so roundtrip works. + _ftpMock.Setup(f => f.Ping()).Returns(true); + _ftpMock.Setup(f => f.UploadFile("/PRG/1", "CONTENT")); + _machine.ConnectAsync().GetAwaiter().GetResult(); + + var prog = new CncProgram { Name = "O0001", Content = "CONTENT" }; + var result = _machine.WriteProgramAsync("1", prog).GetAwaiter().GetResult(); + + Assert.That(result.Success, Is.True); + _ftpMock.Verify(f => f.UploadFile("/PRG/1", "CONTENT"), Times.Once); + } + [Test] public void WriteProgramAsync_EmptyContent_FailsWithoutCallingUploadFile() {