Tree popup: act on the component as it is now, so tree delete is undoable on a device

Right-clicking a tree row selects it, and on a device that reopens the
sheet and rebuilds the DOM after the popup captured the row's element.
Delete then ran on a stale element: the device delete went through but
no undo step was recorded. Popup actions now look the component up by
id when they run.

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

app_state.py

@@ -8,7 +8,7 @@ so later rebinding would only ever be seen by the module that did it.
 import configparser
 import os
 
-VERSION = "0.0.0.147"
+VERSION = "0.0.0.152"
 PROPERTIES_FILE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "editor.properties")
 DEFAULT_GRID_SIZE = 10
 DEFAULT_ADD_TIMESTAMP = True

live_hooks.py

@@ -10,7 +10,7 @@ import binascii
 from tkinter import messagebox
 
 import app_state
-from sox import BUF_ID, type_id_for_slot
+from sox import BUF_ID, CONFIG, STR_ID, type_id_for_slot
 
 
 # Why the last live_* call failed, in words the user can act on. Every failing call sets
@@ -41,8 +41,8 @@ def _session():
 def warn_failure(title, what, detail=None):
     """Tells the user, in a dialog and the status bar, that `what` did not reach the device.
 
-    `detail` defaults to last_error. Live mode has no undo, so the dialog is the only
+    `detail` defaults to last_error. Nothing retries a write that failed, so the dialog
-    record the user gets that the editor and the device may now disagree.
+    is the only record the user gets that the editor and the device may now disagree.
     """
     reason = detail if detail is not None else last_error
     app_state.set_status(f"{what}: {reason}", warn=True)
@@ -230,6 +230,190 @@ def live_invoke(element, slot_name, arg=None):
         return False
 
 
+# ── Undo: read a subtree back, put it back ──────────────────────
+
+def read_subtree(element):
+    """Everything it takes to put `element` and all it holds back on the device.
+
+    Read from the device, not the DOM: the DOM only holds what has been opened, and a
+    placeholder child carries neither config nor children of its own. Requests go one
+    component at a time, at the pace the device sets (see used_kit_names).
+
+    Returns {"tree", "links", "siblings"}, or None with last_error set. A node is
+    {"id", "kit_id", "type_id", "type_name", "name", "values", "element", "children"},
+    where "element" is the DOM element showing it, if any, so a new id can reach it.
+    A link end, and each of "siblings" (the parent's children, in order), is ("node",
+    node), ("elem", element) or ("id", comp_id): a component outside the subtree is
+    followed by its element when it has one, because its id changes if it is itself
+    deleted and put back.
+    """
+    ds = _session()
+    if ds is None:
+        return None
+    comp_id = _comp_id(element)
+    if comp_id is None:
+        return _fail(f"'{element.get('name', '?')}' has no component id.")
+
+    client = ds.sox_client
+    root = app_state.xml_root_element
+    elements_by_id = {}
+    if root is not None:
+        for elem in root.iter("comp"):
+            if elem.get("id") is not None:
+                elements_by_id.setdefault(elem.get("id"), elem)
+
+    nodes_by_id = {}
+
+    def read(comp):
+        if comp.type_name not in app_state.final_components_registry:
+            raise _SnapshotError(f"No kit manifest describes {comp.type_name}, so "
+                                 f"'{comp.name}' could not be put back as it was.")
+        children = ds.load_children(comp.id)
+        if len(children) != len(comp.children):
+            raise _SnapshotError(f"Not every child of '{comp.name}' could be read.")
+        client.update([comp], CONFIG)
+        values = dict(comp.values)
+        # The cache reads every Buf as a Str, which loses bytes of a binary one (a
+        # user's cred); those are read again as what they are.
+        for slot in app_state.final_components_registry[comp.type_name]:
+            if ("c" in slot["flags"] and slot["type"] != "void"
+                    and _wire_type(slot["type"], slot["facets"].get("asStr") is True) == BUF_ID):
+                values[slot["name"]] = client.read_prop(comp.id, slot["slot_id"], BUF_ID)
+        node = {
+            "id": comp.id,
+            "kit_id": comp.kit_id,
+            "type_id": comp.type_id,
+            "type_name": comp.type_name,
+            "name": comp.name,
+            "values": values,
+            "element": elements_by_id.get(str(comp.id)),
+            "children": [],
+        }
+        nodes_by_id[comp.id] = node
+        node["children"] = [read(child) for child in children]
+        return node
+
+    try:
+        top = client.load(comp_id)
+        if top is None:
+            return _fail(f"'{element.get('name', '?')}' could not be read from the device.")
+        tree = read(top)
+
+        links = []
+        seen = set()
+        for cid in list(nodes_by_id):
+            for lnk in client.read_links(cid):
+                key = (lnk.from_comp_id, lnk.from_slot_id, lnk.to_comp_id, lnk.to_slot_id)
+                if key in seen:
+                    continue
+                seen.add(key)
+                links.append((_link_end(lnk.from_comp_id, nodes_by_id, elements_by_id),
+                              lnk.from_slot_id,
+                              _link_end(lnk.to_comp_id, nodes_by_id, elements_by_id),
+                              lnk.to_slot_id))
+    except _SnapshotError as e:
+        return _fail(str(e))
+    except Exception as e:
+        return _fail(f"'{element.get('name', '?')}' could not be read from the device: {e}")
+
+    parent = client.cache_get(top.parent)
+    siblings = [_link_end(cid, nodes_by_id, elements_by_id)
+                for cid in (parent.children if parent is not None else [])]
+    return {"tree": tree, "links": links, "siblings": siblings}
+
+
+class _SnapshotError(Exception):
+    pass
+
+
+def _link_end(comp_id, nodes_by_id, elements_by_id):
+    if comp_id in nodes_by_id:
+        return ("node", nodes_by_id[comp_id])
+    elem = elements_by_id.get(str(comp_id))
+    if elem is not None:
+        return ("elem", elem)
+    return ("id", comp_id)
+
+
+def _link_end_id(end):
+    kind, ref = end
+    if kind == "node":
+        return ref["id"]
+    if kind == "elem":
+        return _comp_id(ref)
+    return ref
+
+
+def recreate_subtree(parent_element, snapshot, build_config):
+    """Adds a subtree from read_subtree back under `parent_element`, with its links.
+
+    The device hands out new ids. Each node, and the DOM element showing it, takes its
+    new id, so whatever refers to them later still finds them. The component goes back
+    to its old place among its siblings.
+
+    `build_config(type_name, device_values=...)` makes an add command's config values.
+    Returns (top_created, complete); when not complete, last_error says what is missing.
+    """
+    ds = _session()
+    if ds is None:
+        return False, False
+    parent_id = _comp_id(parent_element)
+    if parent_id is None:
+        _fail(f"The parent '{parent_element.get('name', '?')}' has no component id.")
+        return False, False
+
+    client = ds.sox_client
+    tree = snapshot["tree"]
+
+    def create(node, under_id):
+        config = build_config(node["type_name"], device_values=node["values"])
+        comp = client.add(under_id, node["kit_id"], node["type_id"], node["name"], config)
+        node["id"] = comp.id
+        if node["element"] is not None:
+            node["element"].set("id", str(comp.id))
+
+    def create_children(node):
+        for child in node["children"]:
+            create(child, node["id"])
+            create_children(child)
+
+    try:
+        create(tree, parent_id)
+    except Exception as e:
+        _fail(f"The device rejected the add of '{tree['name']}': {e}")
+        return False, False
+
+    try:
+        create_children(tree)
+        for from_end, from_slot, to_end, to_slot in snapshot["links"]:
+            client.link(_link_end_id(from_end), from_slot, _link_end_id(to_end), to_slot)
+
+        # The add put it last; the device's order is the order execution runs in.
+        parent = client.cache_get(parent_id)
+        wanted = [_link_end_id(end) for end in snapshot["siblings"]]
+        if (parent is not None and set(wanted) == set(parent.children)
+                and wanted != parent.children):
+            client.reorder(parent_id, wanted)
+        complete = True
+    except Exception as e:
+        _fail(f"'{tree['name']}' is back on the device, but not all of it: {e}")
+        complete = False
+
+    ds.after_add(tree["id"])
+    return True, complete
+
+
+def wire_value(type_id, raw):
+    """A value as the device sent it, in the form encode_value wants for `type_id`."""
+    if type_id == BUF_ID:
+        return bytes(raw) if isinstance(raw, (bytes, bytearray)) else str(raw).encode("utf-8")
+    if type_id == STR_ID:
+        if isinstance(raw, (bytes, bytearray)):
+            return bytes(raw).decode("utf-8", errors="replace").rstrip("\0")
+        return str(raw)
+    return _coerce_value(type_id, str(raw))
+
+
 # ── Helpers ─────────────────────────────────────────────────────
 
 def _comp_id(element):

main.py

@@ -216,6 +216,27 @@ def refresh_all_views():
             close_editor_panel(force=True)
 
 
+def current_element(comp):
+    """The element standing for `comp` in the document now.
+
+    On a device, selecting a tree row opens its sheet, which reads the DOM afresh after
+    a right-click has already picked the row's element. The same component is then a
+    new element with the same id; acting on the old one would leave the document, and
+    undo, holding something other than what the device just did.
+    """
+    root = app_state.xml_root_element
+    if root is None or not app_state.live_mode:
+        return comp
+    if any(element is comp for element in root.iter()):
+        return comp
+    comp_id = comp.get("id")
+    if comp_id is not None:
+        for element in root.iter():
+            if element.tag in ("app", "comp") and element.get("id") == comp_id:
+                return element
+    return comp
+
+
 def _component_in_document(comp):
     root = app_state.xml_root_element
     if root is None:
@@ -531,27 +552,27 @@ def _clear_restoring_flag():
 
 
 def do_undo(event=None):
-    if app_state.live_mode:
-        app_state.set_status("Undo is not available in live mode", warn=True)
-        return "break"
     label = undo.undo()
     if label is None:
         return "break"
-    app_state.window.title(f"{app_state.base_window_title()} — undid {label}")
+    app_state.window.title(f"{_base_title()} — undid {label}")
     return "break"
 
 
 def do_redo(event=None):
-    if app_state.live_mode:
-        app_state.set_status("Redo is not available in live mode", warn=True)
-        return "break"
     label = undo.redo()
     if label is None:
         return "break"
-    app_state.window.title(f"{app_state.base_window_title()} — redid {label}")
+    app_state.window.title(f"{_base_title()} — redid {label}")
     return "break"
 
 
+def _base_title():
+    if app_state.live_mode:
+        return app_state.base_window_title_live()
+    return app_state.base_window_title()
+
+
 def do_save(event=None):
     if app_state.live_mode:
         do_download_to_sax()
@@ -1232,18 +1253,23 @@ def _open_special_view(comp):
 
 
 def on_tree_double_click(event):
-    """Double-click on a tree node opens its special view (or Properties)."""
+    """Double-click on a tree node opens its special view (or Properties).
+
+    Returns "break" so ttk's own double-click does not also toggle the row: only the
+    +/- sign opens or closes a node.
+    """
     tree = app_state.tree
     item_id = tree.identify_row(event.y)
     if not item_id:
-        return
+        return "break"
     comp = app_state.tree_element_map.get(item_id)
     if comp is None:
-        return
+        return "break"
     if tree.parent(item_id) == "":
         show_properties(comp)
     else:
         _open_special_view(comp)
+    return "break"
 
 
 def on_canvas_double_click(event):
@@ -1598,46 +1624,46 @@ def fill_component_popup(menu, comp):
     menu.delete(0, tk.END)
     menu.add_command(
         label="Properties",
-        command=lambda: show_properties(comp),
+        command=lambda: show_properties(current_element(comp)),
     )
     menu.add_command(
         label="Link View",
-        command=lambda: show_link_view(comp),
+        command=lambda: show_link_view(current_element(comp)),
     )
     if ox_page_view.is_ox_page(comp):
         menu.add_command(
             label="Page Editor",
-            command=lambda: show_ox_page_view(comp),
+            command=lambda: show_ox_page_view(current_element(comp)),
         )
     if user_service_view.is_user_service(comp):
         menu.add_command(
             label="User Manager",
-            command=lambda: show_user_service(comp),
+            command=lambda: show_user_service(current_element(comp)),
         )
     menu.add_separator()
     if comp.tag == "comp":
         # The app itself is named by its appName property, not by a component name.
         menu.add_command(
             label="Rename...",
-            command=lambda: open_rename_dialog(comp),
+            command=lambda: open_rename_dialog(current_element(comp)),
         )
     menu.add_command(
         label="Delete Component",
-        command=lambda: delete_component(comp),
+        command=lambda: delete_component(current_element(comp)),
     )
     menu.add_separator()
-    menu.add_command(label="Link Mark", command=lambda: set_link_mark(comp))
+    menu.add_command(label="Link Mark", command=lambda: set_link_mark(current_element(comp)))
 
     marked = app_state.link_mark_element
     if marked is not None:
         mark_name = marked.get("name", "?")
         menu.add_command(
             label=f'Link From "{mark_name}"',
-            command=lambda: open_link_dialog(marked, comp),
+            command=lambda: open_link_dialog(current_element(marked), current_element(comp)),
         )
         menu.add_command(
             label=f'Link To "{mark_name}"',
-            command=lambda: open_link_dialog(comp, marked),
+            command=lambda: open_link_dialog(current_element(comp), current_element(marked)),
         )
 
 
@@ -1766,8 +1792,9 @@ def on_canvas_release(event):
                             "Move", f"{comp_element.get('name', 'The component')} was not "
                             "moved on the device")
                         return
-                if not app_state.live_mode:
+                command_class = (undo.LiveMetaMoveCommand if app_state.live_mode
-                    undo.push(undo.MetaMoveCommand(
+                                 else undo.MetaMoveCommand)
+                undo.push(command_class(
                     [(comp_element, meta_before, raw_x, raw_y)],
                     label=f"move {comp_element.get('name', 'component')}"
                 ))
@@ -2024,6 +2051,12 @@ def record_visit(kind, element):
     """
     global history_index, _history_file_path
 
+    # On a device, undo only reaches back as far as the view that is open. Opening a
+    # sheet reads the DOM afresh from the device, so steps taken before it name elements
+    # that are no longer there; Back and Forward are view changes too.
+    if app_state.live_mode:
+        undo.reset()
+
     if _suppress_history:
         return
 
@@ -2650,6 +2683,8 @@ def _live_tree_expand(comp_id, tree_item_id):
             def _ui():
                 global _restoring_selection
                 app_state.xml_root_element = sax_root
+                # A fresh DOM: undo steps name elements of the one it replaces.
+                undo.reset()
                 sax_file.refresh_treeview_from_dom(sax_root)
                 # Re-open and re-select the node without triggering a wiresheet
                 # re-render — the sheet is already showing the right content and

ox_page_view.py

@@ -554,13 +554,19 @@ class OxPageViewPanel:
 
         # Live: the device first, and the page only shows what the device accepted.
         live_failures = []
-        for element, name, _before, after in changes:
+        applied = []
+        for change in changes:
+            element, name, _before, after = change
             if app_state.live_mode and not live_hooks.live_set_prop_value(element, name, after):
                 live_failures.append(f"{name}: {live_hooks.last_error}")
                 continue
             set_prop_value(element, name, after)
+            applied.append(change)
 
-        if not app_state.live_mode:
+        if app_state.live_mode:
+            if applied:
+                undo.push(undo.LiveSlotValueCommand(applied))
+        else:
             undo.push(undo.SlotValueCommand(changes))
         app_state.mark_dirty()
 

properties.py

@@ -1170,7 +1170,9 @@ class PropertiesPanel:
         # In live mode the device is written first and a value only lands in the document
         # once the device took it, so the panel never shows what the device does not hold.
         live_failures = []
-        for element, prop_name, before, after in changes:
+        applied = []
+        for change in changes:
+            element, prop_name, before, after = change
             if app_state.live_mode and not live_hooks.live_set_prop_value(
                     element, prop_name, after):
                 live_failures.append(f"{prop_name}: {live_hooks.last_error}")
@@ -1182,9 +1184,14 @@ class PropertiesPanel:
                 continue
             if prop_name != "meta":
                 set_prop_value(element, prop_name, after)
-        written = len(changes) - len(live_failures)
+            applied.append(change)
+        written = len(applied)
 
-        if not app_state.live_mode:
+        if app_state.live_mode:
+            # Only what the device took; undo writes it back there.
+            if applied:
+                undo.push(undo.LiveSlotValueCommand(applied))
+        else:
             undo.push(undo.SlotValueCommand(list(changes)))
         app_state.mark_dirty()
 

sax_file.py

@@ -59,9 +59,53 @@ def parse_xml_to_tree(treeview, parent_node, xml_element):
             parse_xml_to_tree(treeview, node_id, child)
 
 
+def _expansion_key(treeview, item, parent_key):
+    """Names a tree row in a way that survives a rebuild of the tree.
+
+    A component's id is stable across edits and across a fresh DOM read from the
+    device; a row without one (the root, or an id-less file component) falls back to
+    its name under its parent's key.
+    """
+    elem = app_state.tree_element_map.get(item)
+    comp_id = elem.get("id") if elem is not None else None
+    if comp_id is not None and treeview.parent(item) != "":
+        return ("id", comp_id)
+    name = elem.get("name", "") if elem is not None else treeview.item(item, "text")
+    return ("path", parent_key, name)
+
+
+def _collect_open_keys(treeview, parent="", parent_key=None, found=None):
+    """Returns the keys of every row that is expanded right now."""
+    if found is None:
+        found = set()
+    for item in treeview.get_children(parent):
+        key = _expansion_key(treeview, item, parent_key)
+        if treeview.item(item, "open"):
+            found.add(key)
+        _collect_open_keys(treeview, item, key, found)
+    return found
+
+
+def _reopen_keys(treeview, open_keys, parent="", parent_key=None):
+    """Expands again the rows that were expanded before the rebuild.
+
+    Setting the open flag in code does not raise <<TreeviewOpen>>, so this never
+    starts a live child load of its own.
+    """
+    for item in treeview.get_children(parent):
+        key = _expansion_key(treeview, item, parent_key)
+        if key in open_keys:
+            treeview.item(item, open=True)
+        _reopen_keys(treeview, open_keys, item, key)
+
+
 def refresh_treeview_from_dom(xml_root):
-    """Rebuilds navigation tree structures using current XML DOM trees."""
+    """Rebuilds navigation tree structures using current XML DOM trees.
+
+    Rows the user had expanded stay expanded; only the user's own "-" closes one.
+    """
     tree = app_state.tree
+    open_keys = _collect_open_keys(tree)
     app_state.tree_element_map.clear()
 
     for item in tree.get_children():
@@ -88,6 +132,8 @@ def refresh_treeview_from_dom(xml_root):
     else:
         parse_xml_to_tree(tree, root_node, xml_root)
 
+    _reopen_keys(tree, open_keys)
+
     if _tree_filled is not None:
         _tree_filled()
 
@@ -859,13 +905,15 @@ def reconcile_schema_kits_before_save():
     return True
 
 
-def build_config_values(full_type, overrides=None):
+def build_config_values(full_type, overrides=None, device_values=None):
     """Builds SOX config_values list from manifest defaults for an add command.
 
     `overrides` maps slot name to the document-form value to send instead of the default.
+    `device_values` maps slot name to a value as the device sent it, for putting back a
+    component exactly as it was; it wins over both.
     Raises ValueError when a value does not fit its slot's type.
     """
-    from live_hooks import _coerce_value, _wire_type
+    from live_hooks import _coerce_value, _wire_type, wire_value
 
     slots = app_state.final_components_registry.get(full_type, [])
     config_values = []
@@ -874,12 +922,15 @@ def build_config_values(full_type, overrides=None):
             continue
         if slot["type"] == "void":
             continue
+        tid = _wire_type(slot["type"], slot["facets"].get("asStr") is True)
+        if device_values and slot["name"] in device_values:
+            config_values.append((tid, wire_value(tid, device_values[slot["name"]])))
+            continue
         val = (overrides or {}).get(slot["name"], slot["default"])
         if val is None:
             val = properties.type_zero(slot["type"])
         if val is None:
             continue
-        tid = _wire_type(slot["type"], slot["facets"].get("asStr") is True)
         config_values.append((tid, _coerce_value(tid, val)))
     return config_values
 
@@ -969,7 +1020,12 @@ def add_component(kit_name, type_name, grid_x, grid_y):
             position_failed = not live_hooks.live_set_prop_value(
                 element, "meta", meta_prop.get("val", "0"))
 
-    if not app_state.live_mode:
+    if app_state.live_mode:
+        undo.push(LiveComponentCommand(
+            parent, index, element, present=True,
+            label=f"add {comp_name}"
+        ))
+    else:
         undo.push(undo.AddComponentCommand(
             parent, index, element,
             label=f"add {comp_name}"
@@ -1012,11 +1068,7 @@ def remove_link(link_element):
     to_ref = link_element.get("to", "")
 
     if app_state.live_mode:
-        from_comp_path, from_slot_name = from_ref.rsplit(".", 1) if "." in from_ref else ("", from_ref)
+        if not live_hooks.live_delete_link(*link_ends(from_ref, to_ref)):
-        to_comp_path, to_slot_name = to_ref.rsplit(".", 1) if "." in to_ref else ("", to_ref)
-        from_elem = find_component_by_path(app_state.xml_root_element, from_comp_path)
-        to_elem = find_component_by_path(app_state.xml_root_element, to_comp_path)
-        if not live_hooks.live_delete_link(from_elem, from_slot_name, to_elem, to_slot_name):
             live_hooks.warn_failure("Delete Link",
                                     f"The link {from_ref} → {to_ref} was not removed from the device")
             return
@@ -1029,7 +1081,12 @@ def remove_link(link_element):
 
     from_slot = from_ref.rsplit(".", 1)[-1] if "." in from_ref else from_ref
     to_slot = to_ref.rsplit(".", 1)[-1] if "." in to_ref else to_ref
-    if not app_state.live_mode:
+    if app_state.live_mode:
+        undo.push(LiveLinkCommand(
+            links_elem, index, link_element, present=False,
+            label=f"delete link {from_slot}→{to_slot}"
+        ))
+    else:
         undo.push(undo.DeleteLinkCommand(
             links_elem, index, link_element,
             label=f"delete link {from_slot}→{to_slot}"
@@ -1050,11 +1107,7 @@ def add_link(from_path_slot, to_path_slot):
         links_elem = ET.SubElement(app_state.xml_root_element, "links")
 
     if app_state.live_mode:
-        from_comp_path, from_slot_name = from_path_slot.rsplit(".", 1) if "." in from_path_slot else ("", from_path_slot)
+        if not live_hooks.live_add_link(*link_ends(from_path_slot, to_path_slot)):
-        to_comp_path, to_slot_name = to_path_slot.rsplit(".", 1) if "." in to_path_slot else ("", to_path_slot)
-        from_elem = find_component_by_path(app_state.xml_root_element, from_comp_path)
-        to_elem = find_component_by_path(app_state.xml_root_element, to_comp_path)
-        if not live_hooks.live_add_link(from_elem, from_slot_name, to_elem, to_slot_name):
             live_hooks.warn_failure("Add Link",
                                     f"The link {from_path_slot} → {to_path_slot} was not made on the device")
             return
@@ -1065,7 +1118,12 @@ def add_link(from_path_slot, to_path_slot):
 
     from_slot = from_path_slot.rsplit(".", 1)[-1] if "." in from_path_slot else from_path_slot
     to_slot = to_path_slot.rsplit(".", 1)[-1] if "." in to_path_slot else to_path_slot
-    if not app_state.live_mode:
+    if app_state.live_mode:
+        undo.push(LiveLinkCommand(
+            links_elem, list(links_elem).index(link_elem), link_elem, present=True,
+            label=f"add link {from_slot}→{to_slot}"
+        ))
+    else:
         undo.push(undo.AddLinkCommand(
             links_elem, link_elem,
             label=f"add link {from_slot}→{to_slot}"
@@ -1091,37 +1149,21 @@ def execute_cascading_deletion(comp_element):
     if not confirm:
         return False
 
+    snapshot = None
     if app_state.live_mode:
+        # Read before it goes: once deleted, the device has nothing left to read back.
+        snapshot = live_hooks.read_subtree(comp_element)
+        unreadable = live_hooks.last_error if snapshot is None else ""
         if not live_hooks.live_delete_component(comp_element):
             live_hooks.warn_failure("Delete Component", f"{comp_name} was not deleted from the device")
             return False
 
-    comp_path = get_component_path(comp_element, app_state.xml_root_element)
-
     all_elements_to_prune = [comp_element] + descendant_comps
     for elem in all_elements_to_prune:
         if elem in app_state.active_components_list:
             app_state.active_components_list.remove(elem)
 
-    # Positions are recorded before anything is removed, so the delete can be reversed.
+    link_records = detach_links_touching(comp_element)
-    link_records = []
-    if app_state.xml_root_element is not None:
-        links_elem = app_state.xml_root_element.find("links")
-        if links_elem is not None:
-            for index, link in enumerate(list(links_elem)):
-                if link.tag != "link":
-                    continue
-                l_from = link.get("from", "")
-                l_to = link.get("to", "")
-
-                from_match = l_from.startswith(comp_path + ".") or l_from.startswith(comp_path + "/")
-                to_match = l_to.startswith(comp_path + ".") or l_to.startswith(comp_path + "/")
-
-                if from_match or to_match:
-                    link_records.append((links_elem, index, link))
-
-            for links_parent, _, link in link_records:
-                links_parent.remove(link)
 
     xml_parent = None
     comp_index = 0
@@ -1131,7 +1173,18 @@ def execute_cascading_deletion(comp_element):
             comp_index = list(xml_parent).index(comp_element)
             xml_parent.remove(comp_element)
 
-    if xml_parent is not None and not app_state.live_mode:
+    if xml_parent is not None and app_state.live_mode:
+        if snapshot is not None:
+            undo.push(LiveComponentCommand(
+                xml_parent, comp_index, comp_element, present=False,
+                snapshot=snapshot, link_records=link_records,
+                label=f"delete {comp_name}"
+            ))
+        else:
+            app_state.set_status(
+                f"{comp_name} was deleted from the device, but this delete cannot be "
+                f"undone: {unreadable}", warn=True)
+    elif xml_parent is not None:
         undo.push(undo.DeleteComponentCommand(
             xml_parent, comp_index, comp_element, link_records,
             label=f"delete {comp_name}"
@@ -1149,3 +1202,156 @@ def execute_cascading_deletion(comp_element):
     refresh_treeview_from_dom(app_state.xml_root_element)
     wiresheet.render_components(app_state.active_components_list)
     return True
+
+
+def link_ends(from_ref, to_ref):
+    """(from_elem, from_slot, to_elem, to_slot) for a link's "path.slot" ends."""
+    from_path, from_slot = from_ref.rsplit(".", 1) if "." in from_ref else ("", from_ref)
+    to_path, to_slot = to_ref.rsplit(".", 1) if "." in to_ref else ("", to_ref)
+    root = app_state.xml_root_element
+    return (find_component_by_path(root, from_path), from_slot,
+            find_component_by_path(root, to_path), to_slot)
+
+
+def detach_links_touching(comp_element):
+    """Takes every <link> into or out of `comp_element`'s subtree out of the DOM.
+
+    Returns (links_parent, index, link) records. Positions are recorded before anything
+    is removed, so putting them back in ascending order lands each on its old index.
+    """
+    root = app_state.xml_root_element
+    if root is None:
+        return []
+    links_elem = root.find("links")
+    if links_elem is None:
+        return []
+
+    comp_path = get_component_path(comp_element, root)
+    link_records = []
+    for index, link in enumerate(list(links_elem)):
+        if link.tag != "link":
+            continue
+        l_from = link.get("from", "")
+        l_to = link.get("to", "")
+        from_match = l_from.startswith(comp_path + ".") or l_from.startswith(comp_path + "/")
+        to_match = l_to.startswith(comp_path + ".") or l_to.startswith(comp_path + "/")
+        if from_match or to_match:
+            link_records.append((links_elem, index, link))
+
+    for links_parent, _, link in link_records:
+        links_parent.remove(link)
+    return link_records
+
+
+class LiveComponentCommand:
+    """A component added to or deleted from the device, undone and redone through Sox.
+
+    Taking it away reads its whole subtree back first (config, children, links) and
+    then deletes it; putting it back adds all of that again. The device gives every
+    component a new id when it comes back, and the DOM elements take those ids on, so
+    later commands holding the same elements still reach the right components.
+    """
+
+    def __init__(self, parent, index, element, present, snapshot=None, link_records=(),
+                 label="add"):
+        # present: whether the component is on the device now. snapshot: from
+        # live_hooks.read_subtree, taken when it was last taken away.
+        self.parent = parent
+        self.index = index
+        self.element = element
+        self.present = present
+        self.snapshot = snapshot
+        self.link_records = list(link_records)
+        self.label = label
+
+    def undo(self):
+        self._flip()
+
+    def redo(self):
+        self._flip()
+
+    def _flip(self):
+        if self.present:
+            self._take_away()
+        else:
+            self._put_back()
+
+    def _take_away(self):
+        name = self.element.get("name", "?")
+        snapshot = live_hooks.read_subtree(self.element)
+        if snapshot is None:
+            raise undo.LiveUndoError(f"{name} was not deleted from the device")
+        if not live_hooks.live_delete_component(self.element):
+            raise undo.LiveUndoError(f"{name} was not deleted from the device")
+
+        self.snapshot = snapshot
+        self.present = False
+        self.link_records = detach_links_touching(self.element)
+        if self.element in list(self.parent):
+            self.index = list(self.parent).index(self.element)
+            self.parent.remove(self.element)
+
+    def _put_back(self):
+        name = self.element.get("name", "?")
+        created, complete = live_hooks.recreate_subtree(
+            self.parent, self.snapshot, build_config_values)
+        if not created:
+            raise undo.LiveUndoError(f"{name} was not put back on the device")
+
+        self.present = True
+        if self.element not in list(self.parent):
+            self.parent.insert(self.index, self.element)
+        for links_parent, index, link in sorted(self.link_records, key=lambda r: r[1]):
+            if link not in list(links_parent):
+                links_parent.insert(index, link)
+
+        ds = app_state.device_session
+        if ds is not None:
+            ds.update_element_from_cache(live_hooks._comp_id(self.element),
+                                         app_state.xml_root_element)
+        if not complete:
+            raise undo.LiveUndoError(f"{name} was not put back on the device in full")
+
+
+class LiveLinkCommand:
+    """A link made or removed on the device, undone and redone through Sox."""
+
+    def __init__(self, links_parent, index, link_element, present, label="add link"):
+        self.links_parent = links_parent
+        self.index = index
+        self.link_element = link_element
+        self.present = present
+        self.label = label
+
+    def undo(self):
+        self._flip()
+
+    def redo(self):
+        self._flip()
+
+    def _flip(self):
+        if self.present:
+            self._unlink()
+        else:
+            self._link()
+
+    def _describe(self):
+        return f"{self.link_element.get('from', '')} → {self.link_element.get('to', '')}"
+
+    def _ends(self):
+        return link_ends(self.link_element.get("from", ""), self.link_element.get("to", ""))
+
+    def _unlink(self):
+        if not live_hooks.live_delete_link(*self._ends()):
+            raise undo.LiveUndoError(f"The link {self._describe()} was not removed from the device")
+        self.present = False
+        if self.link_element in list(self.links_parent):
+            self.index = list(self.links_parent).index(self.link_element)
+            self.links_parent.remove(self.link_element)
+
+    def _link(self):
+        if not live_hooks.live_add_link(*self._ends()):
+            raise undo.LiveUndoError(f"The link {self._describe()} was not made on the device")
+        self.present = True
+        if self.link_element not in list(self.links_parent):
+            self.links_parent.insert(self.index, self.link_element)

undo.py

@@ -1,11 +1,13 @@
 """Undo/redo as a stack of reversible commands.
 
-Commands own the DOM mutation in both directions. Nothing here touches Tk: the view is
+Commands own the DOM mutation in both directions; in live mode they also carry it to the
+device, and raise LiveUndoError when it does not get there. Nothing here touches Tk: the view is
 refreshed through a callback that main registers at startup, which also keeps the
 import graph free of cycles (sax_file and wiresheet both push commands here).
 """
 
 import app_state
+import live_hooks
 import palette
 import schema
 from sax_dom import remove_prop, restore_children, set_prop_value, update_meta_property
@@ -47,11 +49,22 @@ def can_redo():
     return bool(_redo_stack)
 
 
+class LiveUndoError(Exception):
+    """A live command's undo or redo did not fully reach the device.
+
+    Raised with what did not happen; live_hooks.last_error holds why.
+    """
+
+
 def undo():
     if not _undo_stack:
         return None
     command = _undo_stack.pop()
+    try:
         command.undo()
+    except LiveUndoError as failure:
+        _abandon_history("Undo", failure)
+        return None
     _redo_stack.append(command)
     _refresh()
     return command.label
@@ -61,12 +74,27 @@ def redo():
     if not _redo_stack:
         return None
     command = _redo_stack.pop()
+    try:
         command.redo()
+    except LiveUndoError as failure:
+        _abandon_history("Redo", failure)
+        return None
     _undo_stack.append(command)
     _refresh()
     return command.label
 
 
+def _abandon_history(title, failure):
+    """After a step that did not reach the device, the history no longer matches it.
+
+    The device may hold part of the step, so neither direction can be trusted to land
+    where it says. History is dropped rather than replayed against a state it never saw.
+    """
+    reset()
+    _refresh()
+    live_hooks.warn_failure(title, f"{failure}. Undo history was cleared")
+
+
 def mark_saved():
     """Pins the current stack depth as the on-disk state."""
     global _saved_depth
@@ -137,6 +165,44 @@ class MetaMoveCommand:
             update_meta_property(element, old_val, new_x, new_y)
 
 
+def _meta_val(element):
+    meta_prop = element.find("prop[@name='meta']")
+    return meta_prop.get("val") if meta_prop is not None else None
+
+
+class LiveMetaMoveCommand(MetaMoveCommand):
+    """A move on the device: each new position is written there after the DOM takes it.
+
+    A box the device refuses goes back to where the device still has it.
+    """
+
+    def undo(self):
+        before = [(element, _meta_val(element)) for element, *_ in self.changes]
+        super().undo()
+        self._send(before)
+
+    def redo(self):
+        before = [(element, _meta_val(element)) for element, *_ in self.changes]
+        super().redo()
+        self._send(before)
+
+    @staticmethod
+    def _send(before):
+        failures = []
+        for element, old_val in before:
+            new_val = _meta_val(element)
+            if new_val is None or new_val == old_val:
+                continue
+            if not live_hooks.live_set_prop_value(element, "meta", new_val):
+                failures.append(element.get("name", "?"))
+                if old_val is None:
+                    remove_prop(element, "meta")
+                else:
+                    set_prop_value(element, "meta", old_val)
+        if failures:
+            raise LiveUndoError(f"{', '.join(failures)} could not be moved on the device")
+
+
 class SlotValueCommand:
     """One Save from the Properties dialog: every slot it changed, as one undoable step.
 
@@ -167,6 +233,30 @@ class SlotValueCommand:
         self._apply(3)
 
 
+class LiveSlotValueCommand(SlotValueCommand):
+    """Slot edits on the device: each value is written there first, and only a value the
+    device took lands in the DOM, so the views never show what the device does not hold.
+
+    A DOM read from the device carries every slot, so a None side does not come up; one
+    that did would have nothing to send, and only the DOM would change.
+    """
+
+    def _apply(self, value_index):
+        failures = []
+        for change in self.changes:
+            element, prop_name = change[0], change[1]
+            value = change[value_index]
+            if value is None:
+                remove_prop(element, prop_name)
+                continue
+            if not live_hooks.live_set_prop_value(element, prop_name, value):
+                failures.append(f"{element.get('name', '?')}.{prop_name}")
+                continue
+            set_prop_value(element, prop_name, value)
+        if failures:
+            raise LiveUndoError(f"{', '.join(failures)} could not be written to the device")
+
+
 class AddComponentCommand:
     """Placing a component.
 

user_service_view.py

@@ -449,7 +449,10 @@ class _UserDialog:
             ET.SubElement(elem, "prop", name=name, val=val)
 
         if app_state.live_mode:
+            index = len(list(self._service_comp))
             self._service_comp.append(elem)
+            undo.push(sax_file.LiveComponentCommand(
+                self._service_comp, index, elem, present=True, label="add user"))
             app_state.mark_dirty()
             app_state.set_status(f"User '{username}' added to device.")
             return
@@ -485,12 +488,17 @@ class _UserDialog:
 
         if app_state.live_mode:
             failures = []
-            for elem, prop, _old, new in changes:
+            applied = []
+            for change in changes:
+                elem, prop, _old, new = change
                 if live_hooks.live_set_prop_value(elem, prop, new):
                     set_prop_value(elem, prop, new)
+                    applied.append(change)
                     app_state.mark_dirty()
                 else:
                     failures.append(f"{prop}: {live_hooks.last_error}")
+            if applied:
+                undo.push(undo.LiveSlotValueCommand(applied, label="edit user"))
             if failures:
                 live_hooks.warn_failure(
                     "Edit User",
@@ -638,12 +646,24 @@ class UserServicePanel:
             return
 
         if app_state.live_mode:
+            # Read before it goes: once deleted, the device has nothing left to read back.
+            snapshot = live_hooks.read_subtree(user_elem)
+            unreadable = live_hooks.last_error if snapshot is None else ""
             if not live_hooks.live_delete_component(user_elem):
                 live_hooks.warn_failure("Delete User", f"User '{name}' was not deleted from the device")
                 return
+            index = list(self.comp).index(user_elem)
             self.comp.remove(user_elem)
             app_state.mark_dirty()
+            if snapshot is not None:
+                undo.push(sax_file.LiveComponentCommand(
+                    self.comp, index, user_elem, present=False, snapshot=snapshot,
+                    label="delete user"))
                 app_state.set_status(f"User '{name}' deleted from device.")
+            else:
+                app_state.set_status(
+                    f"User '{name}' was deleted from the device, but this delete cannot be "
+                    f"undone: {unreadable}", warn=True)
             self._after_change()
             return
 

wiresheet.py

@@ -33,6 +33,10 @@ WIRE_LANE_GAP = 6
 # knobs, the same way a link leaving the sheet is.
 WIRE_STANDOFF_MAX = 60
 
+# The last turn before a target slot is kept this many grid squares back from the box,
+# so the arrowhead always sits on a straight run of its own instead of on the bend.
+ARRIVE_TURN_GRIDS = 1.5
+
 # Two different wires running horizontally at the same height are indistinguishable, so
 # a free horizontal lane is nudged until it clears any other wire's by this much. The
 # short stubs joining a slot cannot move -- they have to meet their pin -- so a fan-out
@@ -315,6 +319,11 @@ def vertical_run_clearance(cx, y1, y2, boxes):
     return nearest
 
 
+def arrive_turn():
+    """How far in front of a target box the wire makes its last turn, at 1:1."""
+    return max(WIRE_HUG, ARRIVE_TURN_GRIDS * app_state.GRID_SIZE)
+
+
 def channel_candidates(src_box, dst_box, boxes, low, high):
     """Vertical channel positions to try, best first.
 
@@ -322,7 +331,7 @@ def channel_candidates(src_box, dst_box, boxes, low, high):
     that a vertical line stays within WIRE_HUG of a component. The midpoint of the gap
     is kept only as a last resort, for when every hug position is blocked.
     """
-    ordered = [src_box[2] + WIRE_HUG, dst_box[0] - WIRE_HUG]
+    ordered = [src_box[2] + WIRE_HUG, dst_box[0] - arrive_turn()]
     for bx1, _, bx2, _ in boxes:
         ordered.append(bx2 + WIRE_HUG)
         ordered.append(bx1 - WIRE_HUG)
@@ -362,18 +371,22 @@ def free_lane(cx, run_lo, run_hi, boxes, taken, low, high, max_steps=8):
     return None
 
 
-def hug_lane(edge_x, side, run_lo, run_hi, boxes, claimed, max_offset=WIRE_HUG):
+def hug_lane(edge_x, side, run_lo, run_hi, boxes, claimed, max_offset=WIRE_HUG,
+             min_offset=WIRE_MIN_CLEAR):
     """Lane for a run pinned to one box edge, starting WIRE_MIN_CLEAR out from it.
 
     `side` is +1 for a run leaving a box's right edge, -1 for one entering its left
     edge. Offsets inside the hug band are offered first, furthest out of the band
     first so the run keeps clear of its own box; only when every one of those is taken
     does it reach past WIRE_HUG, out to `max_offset`. Returns None when nothing up to
-    that limit clears both the boxes and the runs already placed.
+    that limit clears both the boxes and the runs already placed. `min_offset` raises
+    the floor, and the hug position with it, for a run that must stand further off.
     """
-    offsets = list(range(WIRE_HUG, WIRE_MIN_CLEAR - 1, -1))
+    floor = max(WIRE_MIN_CLEAR, int(math.ceil(min_offset)))
-    if max_offset > WIRE_HUG:
+    hug = max(WIRE_HUG, floor)
-        offsets += list(range(WIRE_HUG + 1, int(max_offset) + 1))
+    offsets = list(range(hug, floor - 1, -1))
+    if max_offset > hug:
+        offsets += list(range(hug + 1, int(max_offset) + 1))
 
     for offset in offsets:
         lane = edge_x + side * offset
@@ -437,7 +450,8 @@ def route_link_path(src_box, src_y, dst_box, dst_y, boxes, taken=None,
     start = (src_box[2], src_y)
     end = (dst_box[0], dst_y)
     leave = (start[0] + LINK_STUB, src_y)
-    arrive = (end[0] - LINK_STUB, dst_y)
+    # The last turn stands arrive_turn() in front of the target, never closer.
+    arrive = (end[0] - max(LINK_STUB, arrive_turn()), dst_y)
 
     low, high = min(leave[0], arrive[0]), max(leave[0], arrive[0])
     run_lo, run_hi = min(src_y, dst_y), max(src_y, dst_y)
@@ -509,7 +523,8 @@ def route_link_path(src_box, src_y, dst_box, dst_y, boxes, taken=None,
                 continue
             in_x = hug_lane(dst_box[0], -1, min(base_cy, dst_y), max(base_cy, dst_y),
                             boxes, claimed + [(out_x, min(src_y, base_cy),
-                                               max(src_y, base_cy))], max_offset)
+                                               max(src_y, base_cy))], max_offset,
+                            min_offset=arrive_turn())
             if in_x is None:
                 continue
 
@@ -1205,12 +1220,12 @@ def wire_demand_by_boundary(column_of_comp):
 def column_gap_for(run_count):
     """Clear space a column gap needs to carry `run_count` vertical wire runs.
 
-    Lanes start WIRE_MIN_CLEAR in from either box edge and stand WIRE_LANE_GAP apart, so
+    Lanes start WIRE_MIN_CLEAR out from the source box and arrive_turn() in front of
-    n of them span 2 * WIRE_MIN_CLEAR + (n - 1) * WIRE_LANE_GAP. One or two runs already
+    the target box, and stand WIRE_LANE_GAP apart, so n of them span
-    fit inside the plain TIDY_GAP -- at 10 and 20 across a 30px gap, both still hugging
+    WIRE_MIN_CLEAR + arrive_turn() + (n - 1) * WIRE_LANE_GAP.
-    a box -- which is why the great majority of gaps stay at exactly TIDY_GAP.
     """
-    return max(TIDY_GAP, 2 * WIRE_MIN_CLEAR + max(0, run_count - 1) * WIRE_LANE_GAP)
+    return max(TIDY_GAP, WIRE_MIN_CLEAR + arrive_turn()
+               + max(0, run_count - 1) * WIRE_LANE_GAP)
 
 
 def plan_columns(columns, column_keys, demand, scale):
@@ -1333,10 +1348,12 @@ def tidy_layout():
     command.redo()          # apply it; the command is the single writer of meta
 
     if app_state.live_mode:
-        # No undo in live mode: each new position goes to the device, and a box the
+        # Each new position goes to the device, and a box the device refused goes back to
-        # device refused goes back to where the device still has it.
+        # where the device still has it. Only the boxes that moved are undoable.
         failures = []
-        for element, old_val, _, _ in changes:
+        moved = []
+        for change in changes:
+            element, old_val, _, _ = change
             new_val = element.find("prop[@name='meta']").get("val", "0")
             if not live_hooks.live_set_prop_value(element, "meta", new_val):
                 failures.append(f"{element.get('name', '?')}: {live_hooks.last_error}")
@@ -1344,6 +1361,10 @@ def tidy_layout():
                     remove_prop(element, "meta")
                 else:
                     set_prop_value(element, "meta", old_val)
+            else:
+                moved.append(change)
+        if moved:
+            undo.push(undo.LiveMetaMoveCommand(moved, label="tidy layout"))
         render_current_sheet()
         if failures:
             live_hooks.warn_failure(