sbbuilder: fixes from review

Toml: a value followed by anything but a comment was parsed as if the
line ended there, so a multi-line string (which opens with an empty one)
or `x = "a" junk` read as valid and set() could write a duplicate or
mangle it. entries() now skips such lines and set() refuses them with the
existing "cannot edit" error. The ponytail comment names the ceilings.

Store: a failed put left Save enabled with a stale draft; it now marks the
key invalid. An empty manifest URL used to unset, which removed the row
being typed in; it now marks the row invalid instead (Setting clears the
mark just before set, so the one made inside set stays). Noted that list
entries with spaces cannot be entered here.

Sbb: refresh() ran an ssh fetch for the desktop page too and let two
fetches overlap, or one run during a save. It is split into reloadDesktop()
and fetch(false), and fetch() returns while busy. A successful config-get
clears a stale VM error, and a missing text reads as empty. The put now
writes and records the draft as sent, so typing during the save is not
marked saved.

SbbVmModule: fields are read-only while busy, a Reload button sits beside
the status line, and the manifest URL field says Remove deletes the row.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

6 files changed, 66 insertions, 26 deletions

diff --git a/desktop/modules/sbbuilder/Sbb.qml b/desktop/modules/sbbuilder/Sbb.qml
index d623423..a34c967 100644
--- a/desktop/modules/sbbuilder/Sbb.qml
+++ b/desktop/modules/sbbuilder/Sbb.qml
@@ -55,10 +55,7 @@ Singleton {
         return ["ssh", "-o", "BatchMode=yes", "-o", "ConnectTimeout=5", ...o, r, "sbb-remote", cmd];
     }
 
-    function refresh() {
-        fv.reload();
-        root.fetch(false);
-    }
+    function reloadDesktop() { fv.reload(); }
 
     function saveDesktop() {
         const d = root.desktop;
@@ -77,7 +74,9 @@ Singleton {
         root.fetch(true);
     }
 
+    // One ssh at a time: a second refresh, or one during a save, is dropped.
     function fetch(thenSave) {
+        if (root.busy) return;
         get.thenSave = thenSave;
         root.busy = true;
         get.running = false;
@@ -117,13 +116,17 @@ Singleton {
             root.vm.defaults = { "": top, manifests: defs.manifests || {} };
             root.vmPath = d.path;
             root.reachable = true;
-            if (!get.thenSave) { root.busy = false; root.vm.loaded(d.text); return; }
-            if (d.text !== root.vm.base) {
+            root.vm.error = "";
+            if (!get.thenSave) { root.busy = false; root.vm.loaded(d.text ?? ""); return; }
+            if ((d.text ?? "") !== root.vm.base) {
                 root.busy = false;
-                root.vm.text = d.text;
+                root.vm.text = d.text ?? "";
                 root.vm.error = "changed on the VM: Discard to load it, then redo";
                 return;
             }
+            // The draft as sent: typing while the save runs must not be
+            // recorded as saved.
+            put.sent = root.vm.draft;
             put.running = false;
             put.running = true;
         }
@@ -131,19 +134,20 @@ Singleton {
 
     Process {
         id: put
+        property string sent: ""
         command: root.ssh("config-set")
         stdinEnabled: true
         stdout: StdioCollector { id: putOut }
         stderr: StdioCollector { id: putErr }
-        onStarted: { put.write(root.vm.draft); put.stdinEnabled = false; }
+        onStarted: { put.write(put.sent); put.stdinEnabled = false; }
         onExited: code => {
             root.busy = false;
             put.stdinEnabled = true;
             let d = null;
             try { d = JSON.parse(putOut.text); } catch (e) {}
             if (code === 0 && d && d.error === undefined) {
-                root.vm.text = root.vm.draft;
-                root.vm.base = root.vm.draft;
+                root.vm.text = put.sent;
+                root.vm.base = put.sent;
             } else root.vm.error = d && d.error ? d.error : (putErr.text.trim() || `ssh exited ${code}`);
         }
     }
diff --git a/desktop/modules/sbbuilder/SbbDesktopModule.qml b/desktop/modules/sbbuilder/SbbDesktopModule.qml
index be4ad22..fc93573 100644
--- a/desktop/modules/sbbuilder/SbbDesktopModule.qml
+++ b/desktop/modules/sbbuilder/SbbDesktopModule.qml
@@ -34,7 +34,7 @@ Module {
         ["builds", "publish", "Publish"], ["hints", "nvchecker_keyfile", "nvchecker keyfile"],
         ["repository", "path", "Repository"]]
     searchItems: fields.map(f => ({ text: f[2], key: f[0] + "/" + f[1] }))
-    function refresh() { Sbb.refresh(); }
+    function refresh() { Sbb.reloadDesktop(); }
 
     page: Component {
         Page {
diff --git a/desktop/modules/sbbuilder/SbbVmModule.qml b/desktop/modules/sbbuilder/SbbVmModule.qml
index 8b9cebc..45c7216 100644
--- a/desktop/modules/sbbuilder/SbbVmModule.qml
+++ b/desktop/modules/sbbuilder/SbbVmModule.qml
@@ -32,7 +32,7 @@ Module {
         ["genreprc", "genreprc"], ["slackrepo_logdir", "slackrepo logs"], ["phantom_deps_file", "Phantom deps"]]
     searchItems: fields.map(f => ({ text: f[1], key: "/" + f[0] }))
         .concat(Sbb.vm.manifests.map(p => ({ text: p + " manifest", key: "manifests/" + p })))
-    function refresh() { Sbb.refresh(); }
+    function refresh() { Sbb.fetch(false); }
 
     page: Component {
         Page {
@@ -50,20 +50,33 @@ Module {
                 spacing: 6
                 readonly property var st: Sbb.vm
                 property string confirming: ""
-                Text {
+                Item {
                     width: col.width
-                    wrapMode: Text.Wrap
-                    text: Sbb.busy ? "Asking the VM..."
-                        : Sbb.reachable ? Sbb.vmPath + " on the VM. The helper checks a save and keeps the old file as .bak."
-                        : "The VM did not answer, so nothing here can change. Start it, then reopen this page."
-                    color: Sbb.reachable || Sbb.busy ? Theme.subtext : Theme.yellow
-                    font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 4 }
+                    height: Math.max(status.implicitHeight, reload.height)
+                    Text {
+                        id: status
+                        anchors { left: parent.left; right: reload.left; rightMargin: 8; verticalCenter: parent.verticalCenter }
+                        wrapMode: Text.Wrap
+                        text: Sbb.busy ? "Asking the VM..."
+                            : Sbb.reachable ? Sbb.vmPath + " on the VM. The helper checks a save and keeps the old file as .bak."
+                            : "The VM did not answer, so nothing here can change. Start it, then Reload."
+                        color: Sbb.reachable || Sbb.busy ? Theme.subtext : Theme.yellow
+                        font { family: Theme.fontFamily; pixelSize: Theme.fontSize - 4 }
+                    }
+                    Button {
+                        id: reload
+                        anchors { right: parent.right; verticalCenter: parent.verticalCenter }
+                        text: "Reload"
+                        enabled: !Sbb.busy
+                        onClicked: Sbb.fetch(false)
+                    }
                 }
 
                 Column {
                     width: col.width
                     spacing: 6
-                    enabled: Sbb.reachable
+                    // Read-only while a save runs too: typing then would be lost or mis-recorded.
+                    enabled: Sbb.reachable && !Sbb.busy
                     opacity: Sbb.reachable ? 1 : 0.5
                     Repeater {
                         model: [
@@ -116,6 +129,7 @@ Module {
                                 id: url
                                 width: parent.width - del.width - 8
                                 store: col.st; section: "manifests"; key: mrow.modelData; label: mrow.modelData
+                                placeholder: "a URL; Remove deletes the row"
                             }
                             Button {
                                 id: del
diff --git a/desktop/modules/sbbuilder/Store.qml b/desktop/modules/sbbuilder/Store.qml
index b4a1ba1..c2afa5d 100644
--- a/desktop/modules/sbbuilder/Store.qml
+++ b/desktop/modules/sbbuilder/Store.qml
@@ -13,7 +13,8 @@ import QtQuick
 import "Toml.js" as T
 
 // One config.toml's draft, the store qtmaildir's Setting and BoolSetting
-// edit. Fields hand it text; it writes the type of the key's default, the
+// edit. ponytail: a list entry holding a space cannot be entered here; edit
+// the file. Fields hand it text; it writes the type of the key's default, the
 // type sb-builder checks on load: a list is words split on spaces, a number
 // is a number, a switch is true or false.
 QtObject {
@@ -45,10 +46,17 @@ QtObject {
     function value(section, key) { return T.get(st.draft, section, key) ?? st.fallback(section, key); }
     function entries(section) { return T.entries(st.draft, section); }
     function put(section, key, v) {
-        try { st.draft = T.set(st.draft, section, key, v); } catch (e) { st.error = e.message; }
+        try { st.draft = T.set(st.draft, section, key, v); } catch (e) {
+            st.error = e.message;
+            st.markInvalid(section + "/" + key, true);  // hold Save back
+        }
     }
     // A field's text; null or "" unsets.
     function set(section, key, t) {
+        // An empty manifest URL is a row still being typed: unsetting would
+        // remove it. Mark it invalid instead; Setting marks the id valid just
+        // before calling set, so this mark, made after, is the one that stays.
+        if (section === "manifests" && t === "") return st.markInvalid(section + "/" + key, true);
         if (t === null || t === "") return st.put(section, key, null);
         const d = st.fallback(section, key);
         st.put(section, key, Array.isArray(d) ? t.split(/\s+/).filter(x => x !== "")
diff --git a/desktop/modules/sbbuilder/Toml.js b/desktop/modules/sbbuilder/Toml.js
index 7fd940a..46b9914 100644
--- a/desktop/modules/sbbuilder/Toml.js
+++ b/desktop/modules/sbbuilder/Toml.js
@@ -17,8 +17,11 @@
 // set again is activated. Top-level keys are section "".
 //
 // ponytail: the TOML these files use, one key per line: strings, integers,
-// booleans, one-line arrays. A value spanning lines makes set() throw rather
-// than write a duplicate key; such a file is edited by hand.
+// booleans, one-line arrays. A value with anything but a comment after it
+// (multi-line strings and arrays included) is refused: entries() skips it and
+// set() throws rather than write a duplicate key; such a file is edited by
+// hand. Also not recognised: CRLF line endings, `[ spaced ]` and `[[array]]`
+// headers, and 'literal' keys.
 
 const HEADER = /^\s*\[([A-Za-z0-9_.-]+)\]\s*(#.*)?$/;
 const KEY = /^(\s*)(#\s*)?("(?:[^"\\]|\\.)*"|[A-Za-z0-9_-]+)\s*=\s*(.*)$/;
@@ -70,6 +73,12 @@ function val(s, i) {
     return { v: v, i: i + m[1].length };
 }
 
+// A whole line's value: the rest of the line must be blank or a comment.
+function whole(s) {
+    const p = val(s, 0);
+    return p && /^\s*(#.*)?$/.test(s.slice(p.i)) ? p : null;
+}
+
 // [{name, h, end}]: the header's line and the line after the section. The
 // top-level keys are a section "" with its header at -1.
 function ranges(lines) {
@@ -91,7 +100,7 @@ function entries(text, section) {
     const out = [];
     for (let i = r.h + 1; i < r.end; i++) {
         const m = KEY.exec(lines[i]);
-        const p = m && !m[2] ? val(m[4], 0) : null;
+        const p = m && !m[2] ? whole(m[4]) : null;
         if (p) out.push({ key: unkey(m[3]), value: p.v });
     }
     return out;
@@ -118,7 +127,7 @@ function set(text, section, key, v) {
         if (!m) continue;
         if (!m[2]) last = i;
         if (unkey(m[3]) !== key) continue;
-        const p = val(m[4], 0);
+        const p = whole(m[4]);
         if (!m[2]) {
             if (!p) throw new Error(key + ": a value this page cannot edit; change it in the file");
             lines[i] = v === null ? m[1] + "# " + lines[i].slice(m[1].length)
diff --git a/desktop/modules/sbbuilder/test_toml.js b/desktop/modules/sbbuilder/test_toml.js
index bb230e9..8378519 100644
--- a/desktop/modules/sbbuilder/test_toml.js
+++ b/desktop/modules/sbbuilder/test_toml.js
@@ -86,6 +86,11 @@ assert.strictEqual(T.get(t, "", "state_dir"), 'a "b" \\ c');
 assert.strictEqual(T.set("[a]\nx = 1\n\n[b]\nx = 2\n", "b", "x", 3), "[a]\nx = 1\n\n[b]\nx = 3\n");
 // A value spanning lines is refused, never duplicated.
 assert.throws(() => T.set("o = [\n  \"a\",\n]\n", "", "o", ["b"]), /cannot edit/);
+// A value followed by junk is refused too: a multi-line string opens with
+// an empty one, and trailing text is no comment.
+assert.throws(() => T.set('x = """\nabc\n"""\n', "", "x", "n"), /cannot edit/);
+assert.throws(() => T.set('x = "a" junk\n', "", "x", "n"), /cannot edit/);
+assert.strictEqual(T.get('x = "a" junk\n', "", "x"), undefined);
 
 // remove deletes the line outright.
 t = T.remove(file, "manifests", "gtk+3");