Merge pull request #1708 from nesquena/fix/1707-workspace-name-click

fix(workspace): preserve single-click open + double-click rename on filename (#1707)

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Hermes Web UI -- Changelog
 
+## [v0.51.5] — 2026-05-05 — single-PR hotfix (#1707)
+
+### Fixed
+
+- **#1707 — single-click on workspace tree filename does nothing.** `static/ui.js` `_renderTreeItems` had `nameEl.onclick=(e)=>e.stopPropagation();` (introduced in #1702 to fix #1698 — clicking the filename was hijacking the dblclick rename handler). Pure stopPropagation swallowed the click entirely, so the row's `el.onclick=async()=>openFile(...)` never fired and clicking the filename did nothing. Fix: replace the pure-barrier with a 300ms-debounced delegator. Single-click on `nameEl` schedules a setTimeout that calls `el.onclick(e)` after the dblclick threshold passes; double-click cancels the pending timer and triggers the existing rename input. Cost: 300ms latency on file-open clicks (acceptable — matches OS dblclick threshold). Also updated `tests/test_workspace_tree_rename.py` to accept both the pre-#1707 (pure stopPropagation) and post-#1707 (debounced delegator) shapes — the original assertion was too narrow. 9 new regression tests in `tests/test_1707_workspace_filename_click.py` (6 source-level + 3 behavioral via Node VM); 7 of 9 fail on master pre-fix, all 9 pass after.
+
 ## [v0.51.4] — 2026-05-05 — 10-PR full-sweep batch
 
 ### Added

ROADMAP.md

@@ -2,7 +2,7 @@
 
 > Web companion to the Hermes Agent CLI. Same workflows, browser-native.
 >
-> Last updated: v0.51.4 (May 5, 2026) — 4503 tests collected — 3-PR follow-up batch (#1671, #1673, #1676)
+> Last updated: v0.51.5 (May 5, 2026) — 4517 tests collected — single-PR hotfix #1707 (workspace filename single-click regression)
 > Test source: `pytest tests/ --collect-only -q`
 > Per-version detail: see [CHANGELOG.md](./CHANGELOG.md)
 

TESTING.md

@@ -1835,7 +1835,7 @@ Bridged CLI sessions:
 
 ---
 
-*Last updated: v0.51.4, May 5, 2026*
+*Last updated: v0.51.5, May 5, 2026*
 *Total automated tests collected: 4503*
 *Regression gate: tests/test_regressions.py*
 *Run: pytest tests/ -v --timeout=60*

static/ui.js

@@ -5702,9 +5702,23 @@ function _renderTreeItems(container, entries, depth){
     // Name
     const nameEl=document.createElement('span');
     nameEl.className='file-name';nameEl.textContent=item.name;nameEl.title=t('double_click_rename');
-    nameEl.onclick=(e)=>e.stopPropagation();
+    // Single-click opens (file) or expand-toggles (dir) but is debounced 300ms so a
+    // double-click can cancel it and trigger rename instead. Without the debounce, the
+    // click bubbles to el.onclick before dblclick can fire — that's #1698. Without the
+    // restored activation, single-click on the filename does nothing — that's #1707.
+    let _nameClickTimer=null;
+    nameEl.onclick=(e)=>{
+      e.stopPropagation();
+      if(_nameClickTimer){clearTimeout(_nameClickTimer);_nameClickTimer=null;}
+      _nameClickTimer=setTimeout(()=>{
+        _nameClickTimer=null;
+        // Delegate to the row's existing single-click handler (openFile / dir toggle).
+        if(typeof el.onclick==='function')el.onclick(e);
+      },300);
+    };
     nameEl.ondblclick=(e)=>{
       e.stopPropagation();
+      if(_nameClickTimer){clearTimeout(_nameClickTimer);_nameClickTimer=null;}
       // For directories, double-click navigates (breadcrumb view)
       if(item.type==='dir'){loadDir(item.path);return;}
       const inp=document.createElement('input');

tests/test_1707_workspace_filename_click.py

@@ -0,0 +1,303 @@
+"""Tests for #1707 — single-click on workspace tree filename does nothing.
+
+Background: #1698 fixed a regression where the filename's dblclick rename
+handler was unreachable because the row's `el.onclick` (openFile) fired
+synchronously on the first click. The fix in #1702 stopped click propagation
+on `nameEl` — but that broke single-click activation entirely (#1707):
+clicking the filename now does nothing, you have to click the icon or row
+whitespace to open the file.
+
+The correct fix preserves both intents:
+
+    let _nameClickTimer = null;
+    nameEl.onclick = (e) => {
+      e.stopPropagation();
+      if (_nameClickTimer) { clearTimeout(_nameClickTimer); _nameClickTimer = null; }
+      _nameClickTimer = setTimeout(() => {
+        _nameClickTimer = null;
+        if (typeof el.onclick === 'function') el.onclick(e);
+      }, 300);
+    };
+    nameEl.ondblclick = (e) => {
+      e.stopPropagation();
+      if (_nameClickTimer) { clearTimeout(_nameClickTimer); _nameClickTimer = null; }
+      // ... existing rename body
+    };
+
+Single-click → 300ms debounce → delegates to the row's `el.onclick` (openFile
+for files, expand-toggle for directories). Double-click → cancels the pending
+timer and triggers rename.
+
+These tests guard the handler shape against regression by static-analyzing
+`static/ui.js` and by driving the patched handler through a Node VM.
+"""
+import json
+import re
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+UI_JS_PATH = REPO_ROOT / "static" / "ui.js"
+NODE = shutil.which("node")
+
+
+def _read_ui_js() -> str:
+    with open(UI_JS_PATH, encoding="utf-8") as f:
+        return f.read()
+
+
+def _name_handler_block() -> str:
+    """Return the source between `nameEl.title=t('double_click_rename')` and the
+    line that appends nameEl to the row (`el.appendChild(nameEl);`).
+    """
+    src = _read_ui_js()
+    start_marker = "nameEl.title=t('double_click_rename');"
+    start = src.find(start_marker)
+    assert start >= 0, "nameEl rename tooltip not found in static/ui.js"
+    end_marker = "el.appendChild(nameEl);"
+    end = src.find(end_marker, start)
+    assert end >= 0, "el.appendChild(nameEl) not found after rename tooltip"
+    return src[start:end + len(end_marker)]
+
+
+# ── Source-level regression locks ─────────────────────────────────────────────
+
+
+class TestNameClickHandlerShape:
+    """Static-analysis assertions on the patched handler shape."""
+
+    def test_nameel_onclick_no_longer_pure_stoppropagation(self):
+        """The pre-fix shape `nameEl.onclick=(e)=>e.stopPropagation();` swallows
+        the click entirely and breaks #1707. The handler must do more than just
+        stop propagation — it must defer activation to `el.onclick`.
+        """
+        block = _name_handler_block()
+        assert not re.search(
+            r"nameEl\.onclick\s*=\s*\(?\s*e\s*\)?\s*=>\s*e\.stopPropagation\(\)\s*;",
+            block,
+        ), (
+            "nameEl.onclick is pure stopPropagation (the #1707 regression); "
+            "it must defer activation to el.onclick after a debounce so single-click "
+            "on the filename still opens the file"
+        )
+
+    def test_nameel_onclick_uses_settimeout_debounce(self):
+        """The fix uses setTimeout to defer activation by ~300ms so dblclick can
+        cancel before the row's openFile fires.
+        """
+        block = _name_handler_block()
+        # Find the nameEl.onclick body (balanced braces) and confirm setTimeout appears in it.
+        m = re.search(r"nameEl\.onclick\s*=\s*\(?\s*e\s*\)?\s*=>\s*\{", block)
+        assert m, "nameEl.onclick assignment not found"
+        start = m.end() - 1
+        depth = 0
+        body = None
+        for i in range(start, len(block)):
+            c = block[i]
+            if c == "{":
+                depth += 1
+            elif c == "}":
+                depth -= 1
+                if depth == 0:
+                    body = block[start:i + 1]
+                    break
+        assert body is not None, "could not find balanced nameEl.onclick body"
+        assert "setTimeout" in body, (
+            "nameEl.onclick must wrap a setTimeout that defers the row's openFile "
+            "by ~300ms so a follow-up dblclick can cancel it. Found body: " + body[:300]
+        )
+        # The debounce duration must be in the dblclick-detection range (200-500ms).
+        delay_m = re.search(r"setTimeout\s*\([^,]+,\s*(\d+)\s*\)", body)
+        assert delay_m, "setTimeout call with numeric delay not found in onclick body"
+        delay = int(delay_m.group(1))
+        assert 200 <= delay <= 500, (
+            f"debounce delay should be in dblclick-detection range (200-500ms); got {delay}ms"
+        )
+
+    def test_nameel_onclick_delegates_to_row_handler(self):
+        """The deferred activation must invoke `el.onclick(...)` (the row's
+        single-click handler) rather than calling openFile directly.
+        """
+        block = _name_handler_block()
+        assert re.search(
+            r"el\.onclick\s*\(",
+            block,
+        ), (
+            "deferred activation must call el.onclick(...) so files use openFile "
+            "and directories use the expand/collapse toggle bound on the row"
+        )
+
+    def test_nameel_ondblclick_cancels_pending_timer(self):
+        """The dblclick handler must clear the pending click-debounce timer."""
+        block = _name_handler_block()
+        m = re.search(
+            r"nameEl\.ondblclick\s*=\s*\(?\s*e\s*\)?\s*=>\s*\{(.*?)\bif\(item\.type==='dir'",
+            block,
+            re.DOTALL,
+        )
+        assert m, "nameEl.ondblclick body not found"
+        ondblclick_head = m.group(1)
+        assert "clearTimeout" in ondblclick_head, (
+            "nameEl.ondblclick must clearTimeout the pending click-debounce timer"
+        )
+
+    def test_row_handlers_still_present(self):
+        """The row's `el.onclick=async()=>openFile(...)` must still be bound."""
+        src = _read_ui_js()
+        assert "el.onclick=async()=>openFile(item.path);" in src, (
+            "row el.onclick must still bind openFile for files"
+        )
+
+    def test_handler_does_not_call_openfile_directly(self):
+        """nameEl.onclick should delegate via el.onclick, not call openFile directly."""
+        block = _name_handler_block()
+        m = re.search(
+            r"nameEl\.onclick\s*=\s*\(?\s*e\s*\)?\s*=>\s*\{(.*?)\};",
+            block,
+            re.DOTALL,
+        )
+        if m:
+            onclick_body = m.group(1)
+            assert "openFile(" not in onclick_body, (
+                "nameEl.onclick must not call openFile directly — delegate to el.onclick(e)"
+            )
+
+
+# ── Behavioral tests via Node VM ──────────────────────────────────────────────
+
+
+pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
+
+
+def _run_node_with_clicks(click_count: int, dblclick_after_first: bool, item_type: str = "file"):
+    """Drive a synthesized click sequence against the patched handler."""
+    handler = _name_handler_block()
+    payload = {
+        "handlerBlock": handler,
+        "clickCount": click_count,
+        "dblclickAfter": dblclick_after_first,
+        "itemType": item_type,
+    }
+    js = (
+        "const params = " + json.dumps(payload) + ";\n"
+        + r"""
+const handlerBlock = params.handlerBlock;
+const clickCount = params.clickCount;
+const dblclickAfter = params.dblclickAfter;
+const itemType = params.itemType;
+
+let openFileCalled = false;
+let dirToggleCalled = false;
+let renameInputMounted = false;
+let pendingTimerClearedByDblclick = false;
+
+const document = {
+  createElement: (tag) => {
+    const el = {
+      tagName: tag.toUpperCase(),
+      className: '', textContent: '', title: '', value: '',
+      onclick: null, ondblclick: null, onkeydown: null, onblur: null,
+      _appended: [], _parent: null,
+      replaceWith(other) { renameInputMounted = true; },
+      appendChild(child) { this._appended.push(child); child._parent = this; },
+      focus() {}, select() {},
+    };
+    return el;
+  },
+};
+
+const nameEl = document.createElement('span');
+const el = {
+  onclick: itemType === 'file'
+    ? (() => { openFileCalled = true; })
+    : (() => { dirToggleCalled = true; }),
+  appendChild() {},
+};
+const item = { type: itemType, path: 'foo/bar.md', name: 'bar.md' };
+const S = { session: { session_id: 'sess-1' }, _expandedDirs: new Set(), _dirCache: {}, currentDir: '.' };
+const t = (key) => key;
+const loadDir = () => {};
+const showToast = () => {};
+const api = async () => ({});
+const setTimeout_ = setTimeout;
+const clearTimeout_ = clearTimeout;
+
+let scheduledTimerId = null;
+const trackedSetTimeout = (cb, ms) => {
+  scheduledTimerId = setTimeout_(cb, ms);
+  return scheduledTimerId;
+};
+const trackedClearTimeout = (id) => {
+  if (id === scheduledTimerId) pendingTimerClearedByDblclick = true;
+  clearTimeout_(id);
+};
+
+const runner = new Function(
+  'nameEl', 'el', 'item', 'S', 't', 'loadDir', 'document', 'showToast', 'api', 'window',
+  'setTimeout', 'clearTimeout',
+  '(()=>{' + handlerBlock + '})();'
+);
+runner(nameEl, el, item, S, t, loadDir, document, showToast, api, {}, trackedSetTimeout, trackedClearTimeout);
+
+const evt = { stopPropagation: () => {} };
+for (let i = 0; i < clickCount; i++) {
+  if (typeof nameEl.onclick === 'function') nameEl.onclick(evt);
+}
+if (dblclickAfter && typeof nameEl.ondblclick === 'function') {
+  nameEl.ondblclick(evt);
+}
+
+setTimeout_(() => {
+  console.log(JSON.stringify({
+    openFileCalled,
+    dirToggleCalled,
+    renameInputMounted,
+    pendingTimerClearedByDblclick,
+  }));
+}, 450);
+"""
+    )
+    r = subprocess.run(
+        [NODE, "-e", js],
+        capture_output=True, text=True, timeout=10,
+    )
+    if r.returncode != 0:
+        raise RuntimeError(f"node failed: {r.stderr}")
+    return json.loads(r.stdout.strip().splitlines()[-1])
+
+
+class TestNameClickBehavior:
+    """End-to-end behavioral tests against the patched handler in a Node VM."""
+
+    def test_single_click_opens_file_after_debounce(self):
+        """Single click on a FILE name → after 300ms debounce → openFile fires."""
+        out = _run_node_with_clicks(click_count=1, dblclick_after_first=False, item_type="file")
+        assert out["openFileCalled"] is True, (
+            f"single click on filename must trigger openFile after debounce; got {out}"
+        )
+        assert out["renameInputMounted"] is False
+        assert out["dirToggleCalled"] is False
+
+    def test_single_click_toggles_dir_after_debounce(self):
+        """Single click on a DIRECTORY name → expand/collapse toggle fires."""
+        out = _run_node_with_clicks(click_count=1, dblclick_after_first=False, item_type="dir")
+        assert out["dirToggleCalled"] is True, (
+            f"single click on directory name must trigger expand/collapse toggle; got {out}"
+        )
+
+    def test_dblclick_cancels_pending_open_and_mounts_rename(self):
+        """Click → dblclick on a file name → rename input mounts, openFile does NOT fire."""
+        out = _run_node_with_clicks(click_count=1, dblclick_after_first=True, item_type="file")
+        assert out["renameInputMounted"] is True, (
+            f"dblclick on filename must mount rename input; got {out}"
+        )
+        assert out["openFileCalled"] is False, (
+            f"dblclick on filename must cancel the pending openFile debounce; got {out}"
+        )
+        assert out["pendingTimerClearedByDblclick"] is True, (
+            f"dblclick must clearTimeout the pending click debounce; got {out}"
+        )

tests/test_workspace_tree_rename.py

@@ -5,18 +5,33 @@ REPO_ROOT = Path(__file__).resolve().parents[1]
 UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8")
 
 
-def test_workspace_file_name_click_stops_before_dblclick_rename():
-    """Clicking a file name must not bubble to the row open handler before dblclick rename."""
+def test_workspace_file_name_click_does_not_immediately_bubble():
+    """Clicking a file name must not synchronously bubble to the row open handler
+    before dblclick can fire. The fix originally landed as pure stopPropagation
+    (#1698), then evolved to a 300ms debounce that delegates to el.onclick (#1707
+    — the pure-stopPropagation form broke single-click activation entirely).
+
+    Either shape satisfies the #1698 invariant. Accept both:
+      - pre-#1707 shape: `nameEl.onclick=(e)=>e.stopPropagation();`
+      - post-#1707 shape: any `nameEl.onclick=(e)=>{...stopPropagation()...setTimeout...}`
+    """
     name_start = UI_JS.index("const nameEl=document.createElement('span');")
     dblclick_idx = UI_JS.index("nameEl.ondblclick=(e)=>", name_start)
-    click_idx = UI_JS.find("nameEl.onclick=(e)=>e.stopPropagation();", name_start, dblclick_idx)
+    block = UI_JS[name_start:dblclick_idx]
 
-    assert click_idx != -1, (
-        "workspace file-tree name span must stop click propagation before its dblclick "
-        "rename handler so the row openFile() click does not win the first click"
+    assert "nameEl.onclick" in block, (
+        "workspace file-tree name span must bind nameEl.onclick to prevent the "
+        "first click of a dblclick from triggering the row's openFile (#1698)"
+    )
+    # The bound handler must call stopPropagation (either the original simple form
+    # or the post-#1707 debounce form that contains stopPropagation in its body).
+    assert "stopPropagation" in block, (
+        "nameEl.onclick must call stopPropagation so the row's el.onclick does not "
+        "fire on the first click of a dblclick (#1698)"
     )
 
 
 def test_workspace_file_row_click_still_opens_file_preview():
-    """Only the name span should swallow clicks; the rest of the file row still opens preview."""
+    """The row-level openFile binding must still exist — the nameEl handler delegates
+    to it (post-#1707) or sits beneath it as a pure barrier (pre-#1707)."""
     assert "el.onclick=async()=>openFile(item.path);" in UI_JS