2026-06-02 08:17:54 +00:00
12 changed files with 122 additions and 31 deletions
--- a/.paul/ISSUES.md
+++ b/.paul/ISSUES.md
@ -182,6 +182,52 @@ Enhancements and known issues discovered during execution and codebase analysis.

 ---

+### ~~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)
+
+---
+
 ## Closed Issues

 | ID | Summary | Phase | Commit |
@ -192,8 +238,10 @@ Enhancements and known issues discovered during execution and codebase analysis.
 | 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-05-25*
-*Sources: STATE.md deferred issues + CONCERNS.md codebase analysis*
+*Last updated: 2026-06-02*
+*Sources: STATE.md deferred issues + CONCERNS.md codebase analysis + field-test docker FTP*
--- 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
            }
        }

-        /// <summary>Build full FTP path. Mitsubishi programs have no extension.</summary>
-        private string ResolvePath(string folder, string name)
+        /// <summary>
+        /// Build full FTP path. Mitsubishi programs have no extension and are addressed
+        /// by their program name/number under <see cref="MitsubishiConnectionConfig.ProgramDirectory"/>.
+        /// 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.
+        /// </summary>
+        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;
        }
    }
 }
--- a/Cnc/NcProgramValidator.cs
+++ b/Cnc/NcProgramValidator.cs
@ -36,12 +36,19 @@ namespace NcProgramManager.Cnc
            string normalized = content.Replace("\r", "");
            string[] lines = normalized.Split('\n');

-            // First-line check (Fanuc/Mitsubishi)
-            if ((_manufacturer == CncManufacturer.Fanuc || _manufacturer == CncManufacturer.Mitsubishi)
-                && lines.Length > 0)
+            // First-line check (Fanuc/Mitsubishi).
+            // FanucProgram.Standardize() prepends a leading '\n' to the program body,
+            // so the O#### header is not necessarily on lines[0]. Scan the first
+            // non-blank line instead of assuming index 0.
+            if (_manufacturer == CncManufacturer.Fanuc || _manufacturer == CncManufacturer.Mitsubishi)
            {
-                string first = lines[0].Trim();
-                if (!FanucFirstLinePattern.IsMatch(first))
+                string first = null;
+                for (int li = 0; li < lines.Length; li++)
+                {
+                    string t = lines[li].Trim();
+                    if (t.Length > 0) { first = t; break; }
+                }
+                if (first == null || !FanucFirstLinePattern.IsMatch(first))
                    errors.Add(new ValidationError(1, "First line must be O#### program number", ValidationSeverity.Error));
            }

--- 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<string>(s => s.EndsWith(".MPF")), It.IsAny<string>()), 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()
        {
--- a/NcProgramManager.Tests/Unit/NcProgramValidatorTests.cs
+++ b/NcProgramManager.Tests/Unit/NcProgramValidatorTests.cs
@ -129,6 +129,18 @@ namespace NcProgramManager.Tests.Unit
            Assert.That(HasErrorContaining(r, "First line"), Is.True);
        }

+        [TestCase(CncManufacturer.Fanuc)]
+        [TestCase(CncManufacturer.Mitsubishi)]
+        public void Validate_LeadingBlankLineBeforeONumber_NoFirstLineError(CncManufacturer mfg)
+        {
+            // ISS-018: FanucProgram.Standardize() prepends a leading '\n', so the
+            // O#### header lands on line[1]. First-line check must scan past blanks.
+            var v = new NcProgramValidator(mfg);
+            var r = v.Validate("O1234", "\nO1234\nG00 X10 Y20\nM30");
+            Assert.That(HasErrorContaining(r, "First line"), Is.False);
+            Assert.That(r.Success, Is.True);
+        }
+
        [TestCase(CncManufacturer.Fanuc,      "O1234",    "O1234\nG00 X10;")]
        [TestCase(CncManufacturer.Mitsubishi, "O1234",    "O1234\nG00 X10;")]
        [TestCase(CncManufacturer.Siemens,    "PROG.MPF", "G00 X10;")]
--- a/field-test/t01_invia_valido.bat
+++ b/field-test/t01_invia_valido.bat
@ -15,7 +15,7 @@ if not exist "%RESULTS%" mkdir "%RESULTS%"
 set LOG=%RESULTS%\t01_invia_valido.log

 echo === T01 INVIO VALIDO === > "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Invia -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%~dp0samples\O0001_valid.nc >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Invia -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%~dp0samples\O0001_valid.nc" >> "%LOG%" 2>&1
 echo EXITCODE=%ERRORLEVEL% >> "%LOG%"
 echo EXITCODE=%ERRORLEVEL%
 echo Log: %LOG%
--- a/field-test/t02_scarica.bat
+++ b/field-test/t02_scarica.bat
@ -17,7 +17,7 @@ set LOG=%RESULTS%\t02_scarica.log
 set OUT=%RESULTS%\downloaded.txt

 echo === T02 SCARICO === > "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%OUT% >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%OUT%" >> "%LOG%" 2>&1
 echo EXITCODE=%ERRORLEVEL% >> "%LOG%"
 echo EXITCODE=%ERRORLEVEL%
 echo File scaricato: %OUT%
--- a/field-test/t03_validatore.bat
+++ b/field-test/t03_validatore.bat
@ -19,7 +19,7 @@ echo === T03 VALIDATORE === > "%LOG%"
 for %%F in (O0001_valid O0002_lowercase O0003_brackets O0004_longblock no_header) do (
  echo. >> "%LOG%"
  echo ----- INVIO %%F ----- >> "%LOG%"
-  "%EXE%" -manufacturer=mitsubishi -azione=Invia -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%~dp0samples\%%F.nc >> "%LOG%" 2>&1
+  "%EXE%" -manufacturer=mitsubishi -azione=Invia -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%~dp0samples\%%F.nc" >> "%LOG%" 2>&1
  echo EXITCODE %%F = !ERRORLEVEL! >> "%LOG%"
 )
 echo Log: %LOG%
--- a/field-test/t04_errori_connessione.bat
+++ b/field-test/t04_errori_connessione.bat
@ -18,17 +18,17 @@ echo === T04 ERRORI CONNESSIONE === > "%LOG%"

 echo. >> "%LOG%"
 echo ----- a) IP inesistente 10.255.255.1 ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=10.255.255.1 -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%RESULTS%\dl_a.txt >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=10.255.255.1 -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%RESULTS%\dl_a.txt" >> "%LOG%" 2>&1
 echo EXITCODE_a=%ERRORLEVEL% >> "%LOG%"

 echo. >> "%LOG%"
 echo ----- b) porta errata 9999 ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=9999 -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%RESULTS%\dl_b.txt >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=9999 -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%RESULTS%\dl_b.txt" >> "%LOG%" 2>&1
 echo EXITCODE_b=%ERRORLEVEL% >> "%LOG%"

 echo. >> "%LOG%"
 echo ----- c) credenziali errate ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=NOUSER -password=NOPASS -path=%PROGNUM% -pathprogramma=%RESULTS%\dl_c.txt >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=NOUSER -password=NOPASS -path=%PROGNUM% -pathprogramma="%RESULTS%\dl_c.txt" >> "%LOG%" 2>&1
 echo EXITCODE_c=%ERRORLEVEL% >> "%LOG%"

 echo Log: %LOG%
--- a/field-test/t05_offset_non_supportati.bat
+++ b/field-test/t05_offset_non_supportati.bat
@ -14,12 +14,12 @@ echo === T05 OFFSET NON SUPPORTATI === > "%LOG%"

 echo. >> "%LOG%"
 echo ----- Scarica con -tooloffset ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%RESULTS%\dl_tool.txt -tooloffset >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%RESULTS%\dl_tool.txt" -tooloffset >> "%LOG%" 2>&1
 echo EXITCODE_tool=%ERRORLEVEL% >> "%LOG%"

 echo. >> "%LOG%"
 echo ----- Scarica con -workzero ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%RESULTS%\dl_wz.txt -workzero >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%RESULTS%\dl_wz.txt" -workzero >> "%LOG%" 2>&1
 echo EXITCODE_wz=%ERRORLEVEL% >> "%LOG%"

 echo Log: %LOG%
--- a/field-test/t06_parametri_mancanti.bat
+++ b/field-test/t06_parametri_mancanti.bat
@ -16,12 +16,12 @@ echo === T06 PARAMETRI MANCANTI === > "%LOG%"

 echo. >> "%LOG%"
 echo ----- a) manca -azione ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -ip=%IP% -porta=%PORT% -pathprogramma=%RESULTS%\x.txt >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -ip=%IP% -porta=%PORT% -pathprogramma="%RESULTS%\x.txt" >> "%LOG%" 2>&1
 echo EXITCODE_a=%ERRORLEVEL% >> "%LOG%"

 echo. >> "%LOG%"
 echo ----- b) manca -ip ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -porta=%PORT% -pathprogramma=%RESULTS%\x.txt >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -porta=%PORT% -pathprogramma="%RESULTS%\x.txt" >> "%LOG%" 2>&1
 echo EXITCODE_b=%ERRORLEVEL% >> "%LOG%"

 echo. >> "%LOG%"
--- a/field-test/t07_roundtrip.bat
+++ b/field-test/t07_roundtrip.bat
@ -17,12 +17,12 @@ echo === T07 ROUNDTRIP === > "%LOG%"

 echo. >> "%LOG%"
 echo ----- 1) INVIO O0001_valid ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Invia -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%~dp0samples\O0001_valid.nc >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Invia -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%~dp0samples\O0001_valid.nc" >> "%LOG%" 2>&1
 echo EXITCODE_invio=%ERRORLEVEL% >> "%LOG%"

 echo. >> "%LOG%"
 echo ----- 2) SCARICO %PROGNUM% ----- >> "%LOG%"
-"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma=%RESULTS%\roundtrip_out.txt >> "%LOG%" 2>&1
+"%EXE%" -manufacturer=mitsubishi -azione=Scarica -ip=%IP% -porta=%PORT% -username=%USER% -password=%PASS% -path=%PROGNUM% -pathprogramma="%RESULTS%\roundtrip_out.txt" >> "%LOG%" 2>&1
 echo EXITCODE_scarico=%ERRORLEVEL% >> "%LOG%"

 echo Log: %LOG%