Ver Fonte

feat: lock a node while somebody is working on it, and show their drags live

Two people on the same workflow could both grab the same node, and the
only sign was the result making no sense afterwards. Now a node being
dragged or configured is held by that editor, and everyone else sees who
has it and what they are doing - "admin moving" on the node itself, with
the reason spelled out on hover.

The locks live on the server. A lock held in a browser that has been
closed is a node nobody can ever touch again, and the only thing that
reliably knows an editor has gone is its connection closing - the same
property that makes presence trustworthy. Leaving the page releases them
too, since navigating away does not close the socket.

A lock is per connection rather than per user: your own second tab is a
different editor with its own copy, and must not be able to drag a node
the first tab is holding.

While a node is held, its position is relayed to everybody watching, so a
drag is visible as it happens rather than appearing somewhere new at the
next save. Positions are relayed, never stored - one in flight is worth
nothing once delivered, and the authoritative one is whatever gets saved.
Throttled to 20 a second, because nobody can see 60.

Only the holder may report a position, checked on the server. Without it
anyone could move anybody's node by sending the message directly.

Three bugs found while building this, each of which made the feature look
like it simply did not work:

- Locks were published only when they changed, so an editor arriving
  after somebody started working saw an unlocked canvas and walked
  straight into a node already taken. They are now sent on subscribe, as
  presence already was.
- That fix first landed in handleUnsubscribe, which has the same two
  lines immediately before the function the patch anchored on.
- Lock state can arrive before the canvas has any nodes - it is published
  the moment this editor subscribes - so it was applied to an empty list
  and then thrown away by the load that followed. The canvas now carries
  an epoch the lock effect re-runs on.

Verified against a control, because a refusal that cannot be told apart
from a broken button proves nothing: opening a node's configuration works
normally, and is refused with "admin is editing this node right now" when
somebody holds it. The badge appears and disappears with the lock, a drag
in one tab moves the node in the other as it happens, and a dropped
connection frees everything that connection held.
fszontagh há 1 mês atrás
pai
commit
2196feaba6

+ 169 - 2
src/webserver/websocket_server.cpp

@@ -162,6 +162,7 @@ void WebSocketServer::onDisconnect(struct lws* wsi) {
     // erased, but the roster can only be republished after - and publishing
     // takes the same lock. So: collect, drop the lock, then publish.
     std::vector<std::string> was_watching;
+    std::string gone_client_id;
 
     {
         std::unique_lock lock(clients_mutex_);
@@ -169,6 +170,7 @@ void WebSocketServer::onDisconnect(struct lws* wsi) {
         auto it = clients_.find(wsi);
         if (it != clients_.end()) {
             LOG_DEBUG("WebSocket client disconnected: {}", it->second->id);
+            gone_client_id = it->second->id;
             for (const auto& sub : it->second->subscriptions) {
                 std::string workflow_id = presenceWorkflowId(sub);
                 if (!workflow_id.empty()) was_watching.push_back(workflow_id);
@@ -178,9 +180,17 @@ void WebSocketServer::onDisconnect(struct lws* wsi) {
         }
     }
 
+    // A lock outlives its holder only if nothing notices they have gone, which
+    // is exactly how an editor ends up with a node nobody can touch again.
+    std::vector<std::string> unlocked;
+    releaseLocksOf(gone_client_id, &unlocked);
+
     for (const auto& workflow_id : was_watching) {
         publishPresence(workflow_id);
     }
+    for (const auto& workflow_id : unlocked) {
+        publishLocks(workflow_id);
+    }
 }
 
 int WebSocketServer::onReceive(struct lws* wsi, const char* data, size_t len) {
@@ -247,6 +257,12 @@ void WebSocketServer::processMessage(WebSocketClient& client, const nlohmann::js
         handleSubscribe(client, message);
     } else if (type == "unsubscribe") {
         handleUnsubscribe(client, message);
+    } else if (type == "lock") {
+        handleLock(client, message);
+    } else if (type == "unlock") {
+        handleUnlock(client, message);
+    } else if (type == "node_moved") {
+        handleNodeMoved(client, message);
     } else if (message_handler_) {
         message_handler_(client.id, message);
     }
@@ -270,6 +286,10 @@ void WebSocketServer::handleAuth(WebSocketClient& client, const nlohmann::json&
         response["type"] = "auth_success";
         response["userId"] = client.user_id;
         response["username"] = client.username;
+        // This connection's own id. A lock is per connection, so an editor has
+        // to be able to tell its own lock from one held by the same person in
+        // another tab.
+        response["clientId"] = client.id;
         sendToClient(client.id, response);
 
         LOG_DEBUG("WebSocket client authenticated: {}", client.id);
@@ -298,6 +318,12 @@ void WebSocketServer::handleSubscribe(WebSocketClient& client, const nlohmann::j
         // waiting for somebody else to come or go.
         std::string workflow_id = presenceWorkflowId(channel);
         if (!workflow_id.empty()) publishPresence(workflow_id);
+
+        // The same for locks, which are otherwise only published when they
+        // change: an editor arriving after somebody started working would see
+        // an unlocked canvas and walk straight into a node already taken.
+        std::string locks_workflow = channelWorkflowId(channel, ".locks");
+        if (!locks_workflow.empty()) publishLocks(locks_workflow);
     }
 }
 
@@ -314,9 +340,9 @@ std::string WebSocketServer::presenceChannel(const std::string& workflow_id) {
     return "workflows." + workflow_id + ".presence";
 }
 
-std::string WebSocketServer::presenceWorkflowId(const std::string& channel) {
+std::string WebSocketServer::channelWorkflowId(const std::string& channel,
+                                              const std::string& suffix) {
     static const std::string prefix = "workflows.";
-    static const std::string suffix = ".presence";
     if (!channel.starts_with(prefix) || !channel.ends_with(suffix)) return "";
 
     // A wildcard subscription is a listener, not somebody with the workflow
@@ -327,6 +353,10 @@ std::string WebSocketServer::presenceWorkflowId(const std::string& channel) {
     return id;
 }
 
+std::string WebSocketServer::presenceWorkflowId(const std::string& channel) {
+    return channelWorkflowId(channel, ".presence");
+}
+
 void WebSocketServer::publishPresence(const std::string& workflow_id) {
     const std::string channel = presenceChannel(workflow_id);
 
@@ -360,6 +390,143 @@ void WebSocketServer::publishPresence(const std::string& workflow_id) {
     broadcast(channel, {{"workflowId", workflow_id}, {"viewers", viewers}});
 }
 
+
+// ---------------------------------------------------------------------------
+// Node locks and live movement
+//
+// While somebody is dragging or configuring a node, everyone else is kept off
+// that one node - not off the workflow. Two people working on different parts
+// of the same flow is the normal case and should stay possible; two people
+// dragging the same node is the one that produces nonsense.
+// ---------------------------------------------------------------------------
+
+void WebSocketServer::handleLock(WebSocketClient& client, const nlohmann::json& message) {
+    if (!client.authenticated) return;
+
+    const std::string workflow_id = message.value("workflowId", "");
+    const std::string node_id = message.value("nodeId", "");
+    const std::string kind = message.value("kind", "editing");
+    if (workflow_id.empty() || node_id.empty()) return;
+
+    bool granted = false;
+    std::string holder_username;
+    {
+        std::lock_guard<std::mutex> lock(locks_mutex_);
+        auto& nodes = locks_[workflow_id];
+        auto it = nodes.find(node_id);
+        if (it == nodes.end() || it->second.client_id == client.id) {
+            // Re-claiming your own lock is how the kind changes from dragging
+            // to editing without a release in between.
+            nodes[node_id] = NodeLock{client.id, client.user_id, client.username, kind};
+            granted = true;
+        } else {
+            holder_username = it->second.username;
+        }
+    }
+
+    if (!granted) {
+        // Said out loud rather than ignored: a click that silently does nothing
+        // reads as the editor being broken.
+        nlohmann::json response;
+        response["type"] = "lock_denied";
+        response["workflowId"] = workflow_id;
+        response["nodeId"] = node_id;
+        response["heldBy"] = holder_username;
+        sendToClient(client.id, response);
+        return;
+    }
+
+    publishLocks(workflow_id);
+}
+
+void WebSocketServer::handleUnlock(WebSocketClient& client, const nlohmann::json& message) {
+    const std::string workflow_id = message.value("workflowId", "");
+    const std::string node_id = message.value("nodeId", "");
+    if (workflow_id.empty() || node_id.empty()) return;
+
+    bool changed = false;
+    {
+        std::lock_guard<std::mutex> lock(locks_mutex_);
+        auto wf = locks_.find(workflow_id);
+        if (wf != locks_.end()) {
+            auto it = wf->second.find(node_id);
+            // Only the holder can let go - otherwise a stale release from an
+            // editor that has already moved on would free somebody else's node.
+            if (it != wf->second.end() && it->second.client_id == client.id) {
+                wf->second.erase(it);
+                changed = true;
+            }
+        }
+    }
+    if (changed) publishLocks(workflow_id);
+}
+
+void WebSocketServer::handleNodeMoved(WebSocketClient& client, const nlohmann::json& message) {
+    if (!client.authenticated) return;
+
+    const std::string workflow_id = message.value("workflowId", "");
+    const std::string node_id = message.value("nodeId", "");
+    if (workflow_id.empty() || node_id.empty()) return;
+
+    // Only the editor holding the node may say where it is. Without this,
+    // anyone could move anybody's node by sending the message directly.
+    {
+        std::lock_guard<std::mutex> lock(locks_mutex_);
+        auto wf = locks_.find(workflow_id);
+        if (wf == locks_.end()) return;
+        auto it = wf->second.find(node_id);
+        if (it == wf->second.end() || it->second.client_id != client.id) return;
+    }
+
+    // Not stored: a position in flight is worth nothing once it has been
+    // delivered, and the authoritative one is whatever gets saved.
+    broadcast("workflows." + workflow_id + ".moves", {
+        {"workflowId", workflow_id},
+        {"nodeId", node_id},
+        {"position", message.value("position", nlohmann::json::object())},
+        {"userId", client.user_id},
+        {"clientId", client.id},
+    });
+}
+
+void WebSocketServer::releaseLocksOf(const std::string& client_id,
+                                     std::vector<std::string>* touched_workflows) {
+    std::lock_guard<std::mutex> lock(locks_mutex_);
+    for (auto& [workflow_id, nodes] : locks_) {
+        bool changed = false;
+        for (auto it = nodes.begin(); it != nodes.end();) {
+            if (it->second.client_id == client_id) {
+                it = nodes.erase(it);
+                changed = true;
+            } else {
+                ++it;
+            }
+        }
+        if (changed && touched_workflows) touched_workflows->push_back(workflow_id);
+    }
+}
+
+void WebSocketServer::publishLocks(const std::string& workflow_id) {
+    nlohmann::json held = nlohmann::json::array();
+    {
+        std::lock_guard<std::mutex> lock(locks_mutex_);
+        auto wf = locks_.find(workflow_id);
+        if (wf != locks_.end()) {
+            for (const auto& [node_id, holder] : wf->second) {
+                held.push_back({
+                    {"nodeId", node_id},
+                    {"userId", holder.user_id},
+                    {"username", holder.username},
+                    {"clientId", holder.client_id},
+                    {"kind", holder.kind},
+                });
+            }
+        }
+    }
+    broadcast("workflows." + workflow_id + ".locks",
+              {{"workflowId", workflow_id}, {"locks", held}});
+}
+
 void WebSocketServer::broadcast(const std::string& channel, const nlohmann::json& data) {
     nlohmann::json message;
     message["channel"] = channel;

+ 24 - 0
src/webserver/websocket_server.hpp

@@ -60,8 +60,17 @@ public:
     // behind - there is no second list that can disagree with the connections.
     void publishPresence(const std::string& workflow_id);
     static std::string presenceWorkflowId(const std::string& channel);
+    static std::string channelWorkflowId(const std::string& channel, const std::string& suffix);
     static std::string presenceChannel(const std::string& workflow_id);
 
+    // Which nodes somebody is currently working on. Held here rather than in
+    // the browsers for the same reason as presence: a lock whose holder has
+    // gone must go with them, and the connection closing is the only thing that
+    // reliably knows.
+    void publishLocks(const std::string& workflow_id);
+    void releaseLocksOf(const std::string& client_id,
+                        std::vector<std::string>* touched_workflows);
+
     // Message handler
     void setMessageHandler(WebSocketMessageHandler handler);
 
@@ -80,6 +89,9 @@ private:
     void handleAuth(WebSocketClient& client, const nlohmann::json& message);
     void handleSubscribe(WebSocketClient& client, const nlohmann::json& message);
     void handleUnsubscribe(WebSocketClient& client, const nlohmann::json& message);
+    void handleLock(WebSocketClient& client, const nlohmann::json& message);
+    void handleUnlock(WebSocketClient& client, const nlohmann::json& message);
+    void handleNodeMoved(WebSocketClient& client, const nlohmann::json& message);
     bool matchesChannel(const std::string& subscription, const std::string& channel);
 
     WebSocketServerConfig config_;
@@ -89,6 +101,18 @@ private:
     std::thread service_thread_;
     std::atomic<bool> running_{false};
 
+    // workflow id -> node id -> who is holding it. A lock is per connection,
+    // not per user: the same person in two tabs is two editors, and the tab
+    // that did not claim the node must not be allowed to drag it either.
+    struct NodeLock {
+        std::string client_id;
+        std::string user_id;
+        std::string username;
+        std::string kind;   // what they are doing: "dragging", "editing"
+    };
+    std::unordered_map<std::string, std::unordered_map<std::string, NodeLock>> locks_;
+    mutable std::mutex locks_mutex_;
+
     std::unordered_map<struct lws*, std::unique_ptr<WebSocketClient>> clients_;
     std::unordered_map<std::string, struct lws*> client_id_map_;
     mutable std::shared_mutex clients_mutex_;

+ 14 - 1
webui/src/components/workflow/WorkflowNode.tsx

@@ -51,6 +51,7 @@ export function resolveOutputs(
 export function WorkflowNode({ data, selected }: NodeProps) {
   const isTrigger = data.isTrigger
   const disabled = data.disabled === true
+  const lockedBy = data.lockedBy as { username: string; kind: string } | undefined
   const execState = data.executionState as NodeExecutionState | undefined
   const outputs: NodeOutput[] = resolveOutputs(data.outputs, data.dynamicOutputs, data.config)
   const inputs: NodeOutput[] = data.inputs?.length
@@ -122,7 +123,7 @@ export function WorkflowNode({ data, selected }: NodeProps) {
           : isTrigger
             ? 'border-green-400'
             : 'border-gray-300 dark:border-slate-600'
-      }`}
+      } ${lockedBy ? 'ring-2 ring-indigo-400 ring-offset-1 dark:ring-offset-slate-900' : ''}`}
       style={getStatusStyle()}
     >
       {/* Input handles at top (not for triggers). A node that names more than
@@ -232,6 +233,18 @@ export function WorkflowNode({ data, selected }: NodeProps) {
           </span>
         )}
 
+        {/* Somebody else is working on this one. Says who and what they are
+            doing, because "you cannot touch this" without a reason is the kind
+            of thing people file bugs about. */}
+        {lockedBy && (
+          <span
+            className="px-1.5 py-0.5 rounded text-[10px] font-medium bg-indigo-100 dark:bg-indigo-900 text-indigo-700 dark:text-indigo-200 whitespace-nowrap"
+            title={`${lockedBy.username} is ${lockedBy.kind === 'dragging' ? 'moving' : 'editing'} this node. It is locked until they are done.`}
+          >
+            {lockedBy.username} {lockedBy.kind === 'dragging' ? 'moving' : 'editing'}
+          </span>
+        )}
+
         {/* Status indicator */}
         {getStatusIcon()}
       </div>

+ 136 - 0
webui/src/hooks/useWorkflowLocks.ts

@@ -0,0 +1,136 @@
+import { useCallback, useEffect, useRef, useState } from 'react'
+import { wsClient } from '../api/client'
+
+/**
+ * Who is holding which node, and where nodes are being dragged right now.
+ *
+ * While somebody is dragging or configuring a node, everyone else is kept off
+ * that one node - not off the workflow. Two people working on different parts
+ * of the same flow is the normal case and should stay possible; two people
+ * dragging the same node is the one that produces nonsense.
+ *
+ * The locks live on the server, because the only thing that reliably knows an
+ * editor has gone is its connection closing. A lock held in a browser that has
+ * been closed is a node nobody can ever touch again.
+ */
+
+export interface NodeLock {
+  nodeId: string
+  userId: string
+  username: string
+  clientId: string
+  /** What they are doing: "dragging" or "editing". */
+  kind: string
+}
+
+// A drag reports a position every frame. Nobody can see 60 updates a second,
+// and sending them costs everybody bandwidth for no benefit.
+const MOVE_INTERVAL_MS = 50
+
+export function useWorkflowLocks(workflowId: string | undefined) {
+  const [locks, setLocks] = useState<Record<string, NodeLock>>({})
+  const [livePositions, setLivePositions] = useState<Record<string, { x: number; y: number }>>({})
+  const [deniedNode, setDeniedNode] = useState<{ nodeId: string; heldBy: string } | null>(null)
+
+  // This connection's own id, so a lock held by this editor is distinguishable
+  // from one held by the same person in a different tab.
+  const clientIdRef = useRef<string | null>(null)
+  const lastMoveSentRef = useRef(0)
+  const heldRef = useRef<Set<string>>(new Set())
+
+  useEffect(() => {
+    if (!workflowId) return
+
+    const lockChannel = `workflows.${workflowId}.locks`
+    const moveChannel = `workflows.${workflowId}.moves`
+
+    const onLocks = (data: any) => {
+      const next: Record<string, NodeLock> = {}
+      for (const l of data?.locks ?? []) next[l.nodeId] = l
+      setLocks(next)
+    }
+
+    const onMove = (data: any) => {
+      // Your own movement is already on your own canvas; echoing it back would
+      // fight the drag in progress.
+      if (data?.clientId && data.clientId === clientIdRef.current) return
+      const p = data?.position
+      if (!data?.nodeId || typeof p?.x !== 'number' || typeof p?.y !== 'number') return
+      setLivePositions((current) => ({ ...current, [data.nodeId]: { x: p.x, y: p.y } }))
+    }
+
+    const onDenied = (data: any) => {
+      if (data?.workflowId !== workflowId) return
+      setDeniedNode({ nodeId: data.nodeId, heldBy: data.heldBy || 'somebody else' })
+    }
+
+    const onIdentity = (data: any) => {
+      if (data?.clientId) clientIdRef.current = data.clientId
+    }
+
+    wsClient.on(lockChannel, onLocks)
+    wsClient.on(moveChannel, onMove)
+    wsClient.on('lock_denied', onDenied)
+    wsClient.on('auth_success', onIdentity)
+    wsClient.subscribe([lockChannel, moveChannel])
+
+    return () => {
+      // Letting go on the way out. The server would release these anyway when
+      // the connection closes, but navigating to another page does not close
+      // it - and leaving a node locked behind you is indistinguishable from a
+      // bug to whoever tries to touch it next.
+      for (const nodeId of heldRef.current) {
+        wsClient.send({ type: 'unlock', workflowId, nodeId })
+      }
+      heldRef.current.clear()
+      wsClient.unsubscribe([lockChannel, moveChannel])
+      wsClient.off(lockChannel, onLocks)
+      wsClient.off(moveChannel, onMove)
+      wsClient.off('lock_denied', onDenied)
+      wsClient.off('auth_success', onIdentity)
+      setLocks({})
+      setLivePositions({})
+      setDeniedNode(null)
+    }
+  }, [workflowId])
+
+  const claim = useCallback((nodeId: string, kind: 'dragging' | 'editing') => {
+    if (!workflowId) return
+    heldRef.current.add(nodeId)
+    wsClient.send({ type: 'lock', workflowId, nodeId, kind })
+  }, [workflowId])
+
+  const release = useCallback((nodeId: string) => {
+    if (!workflowId) return
+    heldRef.current.delete(nodeId)
+    wsClient.send({ type: 'unlock', workflowId, nodeId })
+    setLivePositions((current) => {
+      if (!(nodeId in current)) return current
+      const next = { ...current }
+      delete next[nodeId]
+      return next
+    })
+  }, [workflowId])
+
+  const reportMove = useCallback((nodeId: string, position: { x: number; y: number }) => {
+    if (!workflowId) return
+    const now = performance.now()
+    if (now - lastMoveSentRef.current < MOVE_INTERVAL_MS) return
+    lastMoveSentRef.current = now
+    wsClient.send({ type: 'node_moved', workflowId, nodeId, position })
+  }, [workflowId])
+
+  /** A lock held by a different editor - your own claim does not lock you out. */
+  const lockedByOther = useCallback(
+    (nodeId: string): NodeLock | null => {
+      const held = locks[nodeId]
+      if (!held) return null
+      return held.clientId === clientIdRef.current ? null : held
+    },
+    [locks]
+  )
+
+  const dismissDenied = useCallback(() => setDeniedNode(null), [])
+
+  return { locks, lockedByOther, livePositions, claim, release, reportMove, deniedNode, dismissDenied }
+}

+ 61 - 1
webui/src/pages/WorkflowEditorPage.tsx

@@ -31,6 +31,7 @@ import { useCallback, useEffect, useState, useRef, useMemo } from 'react'
 import { AlertTriangle } from 'lucide-react'
 import { wsClient } from '../api/client'
 import { useWorkflowPresence } from '../hooks/useWorkflowPresence'
+import { useWorkflowLocks } from '../hooks/useWorkflowLocks'
 import { diffWorkflows, mergeWorkflows, summariseChanges, type Change } from '../utils/workflowDiff'
 import type { AvailableField } from '../components/ConditionBuilder'
 import type { FieldInfo } from '../components/AvailableDataPanel'
@@ -358,6 +359,11 @@ function WorkflowEditorInner() {
   // Set only by the merge path: the graph about to be loaded is not the stored
   // one, so the canvas has unsaved work the moment it appears.
   const loadedIsUnsavedRef = useRef(false)
+  // Bumped every time the canvas is rebuilt from a record. Lock state can
+  // arrive before there are any nodes to put it on - it is published the moment
+  // this editor subscribes - and without something to re-run on, that state
+  // would be applied to an empty canvas and then thrown away by the load.
+  const [graphEpoch, setGraphEpoch] = useState(0)
   const [selectedNodeData, setSelectedNodeData] = useState<SelectedNodeData | null>(null)
   const [showNodeConfig, setShowNodeConfig] = useState(false)
   const [editingConfig, setEditingConfig] = useState<Record<string, any>>({})
@@ -585,6 +591,10 @@ function WorkflowEditorInner() {
   const loadedVersionRef = useRef<number | null>(null)
   const { others: otherViewers, myOtherTabs, savedByOther, dismissSavedByOther } =
     useWorkflowPresence(id, loadedVersionRef)
+  const {
+    lockedByOther, livePositions, claim: claimNode, release: releaseNode,
+    reportMove, deniedNode, dismissDenied,
+  } = useWorkflowLocks(id)
   // The save handler needs the latest value without being rebuilt for it.
   const savedByOtherRef = useRef(savedByOther)
   savedByOtherRef.current = savedByOther
@@ -1476,9 +1486,45 @@ function WorkflowEditorInner() {
       // out.
       setHasChanges(loadedIsUnsavedRef.current)
       loadedIsUnsavedRef.current = false
+      setGraphEpoch((n) => n + 1)
     }
   }, [workflow, nodeDefs, nodeDefsMap, setNodes, setEdges])
 
+  // Put the lock state and anybody else's in-flight drag onto the canvas.
+  // Separate from loading, because these arrive while somebody is working and
+  // must not disturb anything else about the node.
+  useEffect(() => {
+    setNodes((nds) => {
+      let touched = false
+      const next = nds.map((n) => {
+        const held = lockedByOther(n.id)
+        const live = livePositions[n.id]
+        const heldChanged = (n.data.lockedBy?.clientId ?? null) !== (held?.clientId ?? null) ||
+                            (n.data.lockedBy?.kind ?? null) !== (held?.kind ?? null)
+        const moved = !!live && (n.position.x !== live.x || n.position.y !== live.y)
+        if (!heldChanged && !moved) return n
+        touched = true
+        return {
+          ...n,
+          // Locked by somebody else means locked: dragging it here would send
+          // the two editors into a tug of war neither can win.
+          draggable: !held,
+          position: moved ? live! : n.position,
+          data: heldChanged ? { ...n.data, lockedBy: held ?? undefined } : n.data,
+        }
+      })
+      return touched ? next : nds
+    })
+  }, [lockedByOther, livePositions, setNodes, graphEpoch])
+
+  // Asking for a node somebody else is holding is refused, and saying so is the
+  // difference between a locked node and a broken editor.
+  useEffect(() => {
+    if (!deniedNode) return
+    showToast('error', `That node is being edited by ${deniedNode.heldBy} right now`)
+    dismissDenied()
+  }, [deniedNode, dismissDenied, showToast])
+
   // Update nodes with execution state (uses effectiveExecutionState for viewing pinned executions)
   useEffect(() => {
     setNodes((nds) =>
@@ -1661,6 +1707,14 @@ function WorkflowEditorInner() {
 
   // Open node config
   const openNodeConfig = useCallback((nodeId: string) => {
+    // Refused before it opens, rather than opening and having the save fight
+    // whoever is already in there.
+    const held = lockedByOther(nodeId)
+    if (held) {
+      showToast('error', `${held.username} is ${held.kind === 'dragging' ? 'moving' : 'editing'} this node right now`)
+      setContextMenu(null)
+      return
+    }
     const node = nodes.find((n) => n.id === nodeId)
     if (node) {
       setSelectedNodeData({
@@ -1671,9 +1725,10 @@ function WorkflowEditorInner() {
       })
       setEditingConfig(node.data.config || {})
       setShowNodeConfig(true)
+      claimNode(nodeId, 'editing')
     }
     setContextMenu(null)
-  }, [nodes])
+  }, [nodes, claimNode, lockedByOther, showToast])
 
   // Build a canvas node from a definition. Shared by adding, dropping and
   // pasting, so a node arrives the same way however it got here.
@@ -2386,6 +2441,7 @@ function WorkflowEditorInner() {
     }
 
     setHasChanges(true)
+    if (selectedNodeData?.id) releaseNode(selectedNodeData.id)
     setShowNodeConfig(false)
     setSelectedNodeData(null)
     setShowDataPanel(false)
@@ -3052,6 +3108,9 @@ function WorkflowEditorInner() {
             panActivationKeyCode="Control"
             // Disable editing when viewing execution snapshot
             nodesDraggable={!isViewingExecution}
+            onNodeDragStart={(_e, node) => claimNode(node.id, 'dragging')}
+            onNodeDrag={(_e, node) => reportMove(node.id, node.position)}
+            onNodeDragStop={(_e, node) => releaseNode(node.id)}
             nodesConnectable={!isViewingExecution}
             elementsSelectable={!isViewingExecution}
             onNodesChange={isViewingExecution ? undefined : (changes) => {
@@ -3216,6 +3275,7 @@ function WorkflowEditorInner() {
           onConfigChange={setEditingConfig}
           onSave={saveNodeConfig}
           onClose={() => {
+            if (selectedNodeData?.id) releaseNode(selectedNodeData.id)
             setShowNodeConfig(false)
             setSelectedNodeData(null)
             setShowDataPanel(false)