fix(sidebar): preserve in-progress session rename across list rebroadcasts (lr-16b88d)

lib/public/modules/sidebar-sessions.js

@@ -53,12 +53,23 @@ var armedDeleteTimer = null;
 
 // Active inline-rename tracking (session or loop). A full session-list
 // rebuild (innerHTML = "") detaches the rename <input> from the DOM without
-// firing a real user "blur" — the browser's synthetic blur-on-removal still
-// invokes commitRename(), but it operates on a textSpan that renderSessionList
-// is about to throw away, so the typed title is silently discarded. Tracking
-// the active commit/cancel lets renderSessionList settle it cleanly (using the
-// input's *current* value) before tearing the list down, instead of losing it.
-var activeRename = null; // { commit, cancel } or null
+// firing a real user "blur" — but the browser DOES synthesize a blur event
+// on detach, and the input's own blur listener is still attached at that
+// instant. Committing on every rebuild (the original design here) turned
+// out to be wrong: an actively-streaming session broadcasts session_list
+// many times per turn, so the rename committed within a keystroke or two,
+// sending a partial title as a real rename_session (durable write +
+// permanent titleManuallySet=true — see lr-16b88d). `suspend()` captures
+// the in-progress edit (value + caret) AND settles it as suspended (same
+// short-circuit `settled` flag commit/cancel use) — no commit, no cancel,
+// no WS send — so the detach-induced synthetic blur that fires the instant
+// the rebuild's innerHTML="" runs is a no-op on the old input. renderSessionList
+// then reassigns activeRename to null and, if a real rebuild ran, re-opens
+// the rename afterward with the captured snapshot (see the suspendedRename
+// re-open block at the end of renderSessionList). Precedent: scheduler.js's
+// `cancelled` short-circuit flag / detach-as-blur handling (lr-fb49-E/F
+// family) — suspend() is that same pattern applied to preserve-not-drop.
+var activeRename = null; // { type, id, currentTitle, commit, cancel, suspend } or null
 
 export function openResumePicker() {
   openResumePickerModal();
@@ -747,40 +758,61 @@ function showLoopCtxMenu(anchorBtn, loopId, loopName, childCount) {
 
 // --- Inline rename ---
 
-function startInlineRename(sessionId, currentTitle) {
+// resume: optional { value, selectionStart, selectionEnd } captured by a
+// prior suspend() — restores the in-progress edit instead of starting fresh
+// from currentTitle. Passed by renderSessionList() when re-opening a rename
+// after a real rebuild tore the previous <input> down.
+//
+// LIFECYCLE GUARD (lr-16b88d PEACHES finding): `settled` alone is not
+// enough once suspend() exists. A rebuild's innerHTML="" detaches this
+// <input> without a real user blur, but the browser still SYNTHESIZES a
+// blur event on detach — and that listener (registered below) is still
+// attached at that moment. Without a guard, that synthetic blur fires
+// commitRename() and ships the suspended partial edit as a real
+// rename_session, exactly the bug this task exists to fix. suspend() must
+// therefore flip `settled` itself (same short-circuit commit/cancel use),
+// so the detach-induced blur that fires immediately afterward is a no-op —
+// mirrors scheduler.js:609-621's `cancelled` short-circuit / detach-as-blur
+// precedent.
+function startInlineRename(sessionId, currentTitle, resume) {
   var el = getSessionListEl().querySelector('.session-item[data-session-id="' + sessionId + '"]');
   if (!el) return;
   var textSpan = el.querySelector(".session-item-text");
   if (!textSpan) return;
 
   // Settle (not silently drop) any rename already in progress elsewhere in
-  // the list before starting a new one.
+  // the list before starting a new one. This is a real user-driven "start a
+  // different rename" action, not a rebuild-induced detach — commit here is
+  // correct (matches existing blur-to-commit semantics for switching target).
   if (activeRename) activeRename.commit();
 
   var input = document.createElement("input");
   input.type = "text";
   input.className = "session-rename-input";
-  input.value = currentTitle || "New Session";
+  input.value = resume ? resume.value : (currentTitle || "New Session");
 
   var originalHtml = textSpan.innerHTML;
   textSpan.innerHTML = "";
   textSpan.appendChild(input);
   input.focus();
-  input.select();
+  if (resume) {
+    input.setSelectionRange(resume.selectionStart, resume.selectionEnd);
+  } else {
+    input.select();
+  }
 
   var settled = false;
 
   function commitRename() {
     if (settled) return;
     settled = true;
-    activeRename = null;
+    if (activeRename && activeRename.commit === commitRename) activeRename = null;
     var newTitle = input.value.trim();
     if (newTitle && newTitle !== currentTitle && getWs() && store.get('connected')) {
       getWs().send(JSON.stringify({ type: "rename_session", id: sessionId, title: newTitle }));
     }
     // Restore text (server will send updated session_list). Guard against the
-    // textSpan already having been detached/replaced by a rebuild that ran
-    // this same settle path via activeRename.commit() at its top.
+    // textSpan already having been detached/replaced by a rebuild.
     if (getSessionListEl().contains(textSpan)) {
       textSpan.innerHTML = originalHtml;
       if (newTitle && newTitle !== currentTitle) {
@@ -792,21 +824,43 @@ function startInlineRename(sessionId, currentTitle) {
   function cancelRename() {
     if (settled) return;
     settled = true;
-    activeRename = null;
+    if (activeRename && activeRename.commit === commitRename) activeRename = null;
     if (getSessionListEl().contains(textSpan)) textSpan.innerHTML = originalHtml;
   }
 
+  // Capture the live edit AND settle it as suspended — no commit, no cancel,
+  // no WS send, but `settled` flips to true so the synthetic blur the
+  // upcoming innerHTML="" teardown fires against this exact input becomes a
+  // no-op (see the LIFECYCLE GUARD comment above). Does not clear
+  // module-level `activeRename` itself — the caller (renderSessionList)
+  // owns reassigning that slot once it knows whether a real rebuild ran and,
+  // if so, once the re-opened rename has installed its own activeRename.
+  function suspendRename() {
+    if (settled) return null;
+    settled = true;
+    return { value: input.value, selectionStart: input.selectionStart, selectionEnd: input.selectionEnd };
+  }
+
   input.addEventListener("keydown", function (e) {
     if (e.key === "Enter") { e.preventDefault(); commitRename(); }
     if (e.key === "Escape") { e.preventDefault(); cancelRename(); }
   });
   input.addEventListener("blur", commitRename);
   input.addEventListener("click", function (e) { e.stopPropagation(); });
 
-  activeRename = { commit: commitRename, cancel: cancelRename };
+  activeRename = {
+    type: "session",
+    id: sessionId,
+    currentTitle: currentTitle,
+    commit: commitRename,
+    cancel: cancelRename,
+    suspend: suspendRename,
+  };
 }
 
-function startLoopInlineRename(loopId, currentName) {
+// resume: see startInlineRename's matching parameter. See startInlineRename's
+// LIFECYCLE GUARD comment — identical reasoning applies here.
+function startLoopInlineRename(loopId, currentName, resume) {
   var el = getSessionListEl().querySelector('.session-loop-group[data-loop-id="' + loopId + '"]');
   if (!el) return;
   var textSpan = el.querySelector(".session-item-text");
@@ -817,20 +871,24 @@ function startLoopInlineRename(loopId, currentName) {
   var input = document.createElement("input");
   input.type = "text";
   input.className = "session-rename-input";
-  input.value = currentName || "Loop";
+  input.value = resume ? resume.value : (currentName || "Loop");
 
   var originalHtml = textSpan.innerHTML;
   textSpan.innerHTML = "";
   textSpan.appendChild(input);
   input.focus();
-  input.select();
+  if (resume) {
+    input.setSelectionRange(resume.selectionStart, resume.selectionEnd);
+  } else {
+    input.select();
+  }
 
   var settled = false;
 
   function commitRename() {
     if (settled) return;
     settled = true;
-    activeRename = null;
+    if (activeRename && activeRename.commit === commitRename) activeRename = null;
     var newName = input.value.trim();
     if (newName && newName !== currentName && getWs() && store.get('connected')) {
       getWs().send(JSON.stringify({ type: "loop_registry_rename", id: loopId, name: newName }));
@@ -848,18 +906,32 @@ function startLoopInlineRename(loopId, currentName) {
   function cancelRename() {
     if (settled) return;
     settled = true;
-    activeRename = null;
+    if (activeRename && activeRename.commit === commitRename) activeRename = null;
     if (getSessionListEl().contains(textSpan)) textSpan.innerHTML = originalHtml;
   }
 
+  // See startInlineRename's suspendRename — same settle-without-committing.
+  function suspendRename() {
+    if (settled) return null;
+    settled = true;
+    return { value: input.value, selectionStart: input.selectionStart, selectionEnd: input.selectionEnd };
+  }
+
   input.addEventListener("keydown", function (e) {
     if (e.key === "Enter") { e.preventDefault(); commitRename(); }
     if (e.key === "Escape") { e.preventDefault(); cancelRename(); }
   });
   input.addEventListener("blur", commitRename);
   input.addEventListener("click", function (e) { e.stopPropagation(); });
 
-  activeRename = { commit: commitRename, cancel: cancelRename };
+  activeRename = {
+    type: "loop",
+    id: loopId,
+    currentTitle: currentName,
+    commit: commitRename,
+    cancel: cancelRename,
+    suspend: suspendRename,
+  };
 }
 
 // --- Date grouping / highlighting ---
@@ -1275,30 +1347,55 @@ function _fingerprintSessions(list, expanded, expandedRuns) {
 export function renderSessionList(sessions) {
   if (sessions) cachedSessions = sessions;
 
-  // A full rebuild below (innerHTML = "") detaches the in-progress rename
-  // <input> / armed-delete "x" button without a real user action. Settle
-  // both cleanly first: commit the rename using its current value (matching
-  // existing blur-to-commit behavior) and clear the armed-delete affordance
-  // so the rebuilt button doesn't silently delete on the next single click.
-  if (activeRename) activeRename.commit();
-  clearArmedSessionDelete();
-
-  // Skip full rebuild when session data hasn't changed. Server often
-  // re-broadcasts session_list after unrelated events (session switches,
-  // WS reconnects, presence updates). Each rebuild does innerHTML="" on the
-  // entire sidebar list + re-creates all elements + calls refreshIcons().
-  // Include search state so filter changes always trigger a rebuild.
+  // Compute the fingerprint FIRST, before touching activeRename at all.
+  // suspend() settles the rename's input (see its LIFECYCLE GUARD comment) —
+  // it must only be called when a real rebuild is about to tear that input
+  // down. Calling it unconditionally (including on a no-op render, where the
+  // input stays live in the DOM untouched) would settle an edit that was
+  // never actually interrupted, silently disabling its real blur/Enter/
+  // Escape handling. This ordering is the fix for the lr-16b88d PEACHES
+  // finding that the original version suspended before knowing whether a
+  // rebuild would run.
   var fp = _fingerprintSessions(cachedSessions, expandedLoopGroups, expandedLoopRuns) +
     "|sq:" + searchQuery + "|sm:" + (searchMatchIds ? searchMatchIds.size : "null");
   if (fp === _sessionListFingerprint) {
-    // Still need to refresh mobile sheet and search UI on null calls
+    // No-op render: nothing is going to be torn down, so the active rename's
+    // input stays live in the DOM untouched — leave activeRename alone.
+    // Still need to refresh mobile sheet and search UI on null calls.
     if (refreshMobileChatSheet) refreshMobileChatSheet();
     syncHeaderSearchUi();
     if (updatePageTitle) updatePageTitle();
     return;
   }
   _sessionListFingerprint = fp;
 
+  // A real rebuild below (innerHTML = "") detaches the in-progress rename
+  // <input> from the DOM without a real user action. Force-committing it
+  // here (the original design) sent a partial title as a real rename_session
+  // on nearly every broadcast from an actively-streaming session — see
+  // lr-16b88d. Instead, capture the in-progress edit via suspend(), which
+  // both snapshots {value, selectionStart, selectionEnd} AND settles the
+  // rename as suspended (no commit, no cancel, no WS send) so the detach-
+  // induced synthetic blur that fires the instant innerHTML="" runs below is
+  // a no-op on the OLD input. Clear the module-level activeRename slot
+  // immediately after: this rebuild is the only owner of the decision to
+  // re-open it, and no code between here and the re-open below may treat a
+  // stale reference to the just-torn-down rename as still committable.
+  var suspendedRename = null;
+  if (activeRename) {
+    var snapshot = activeRename.suspend();
+    if (snapshot) {
+      suspendedRename = {
+        type: activeRename.type,
+        id: activeRename.id,
+        currentTitle: activeRename.currentTitle,
+        snapshot: snapshot,
+      };
+    }
+    activeRename = null;
+  }
+  clearArmedSessionDelete();
+
   // If mobile chat sheet is open, refresh it
   if (refreshMobileChatSheet) refreshMobileChatSheet();
 
@@ -1423,6 +1520,20 @@ export function renderSessionList(sessions) {
   // no-op when schedules are outside the 3-minute window or the timer is
   // already running, so calling it here is safe and low-cost.
   startCountdownTimer();
+
+  // Re-open the in-progress rename this rebuild just tore down, restoring
+  // the typed value + caret (lr-16b88d). If the target session/loop no
+  // longer exists post-rebuild (deleted, or filtered out by a new search),
+  // startInlineRename()/startLoopInlineRename() find no matching element and
+  // no-op, and activeRename is left null — the edit is silently dropped in
+  // that case, but it was already unaddressable, not force-committed.
+  if (suspendedRename) {
+    if (suspendedRename.type === "loop") {
+      startLoopInlineRename(suspendedRename.id, suspendedRename.currentTitle, suspendedRename.snapshot);
+    } else {
+      startInlineRename(suspendedRename.id, suspendedRename.currentTitle, suspendedRename.snapshot);
+    }
+  }
 }
 
 // --- Search results ---