fix: workspace panel close button — no duplicate X on desktop, mobile X respects file preview (#414)

* fix: workspace panel close button — no duplicate X on desktop, mobile X respects file preview

Two bugs fixed in the workspace right panel:

1. Duplicate X on desktop (bug): #btnClearPreview (the X icon) was always
   visible alongside #btnCollapseWorkspacePanel (the chevron), producing two
   close controls at once. Fixed in syncWorkspacePanelUI() — on desktop, the X
   is now hidden when no file preview is open (display:none), and only shown
   when the user is viewing a file. The chevron remains as the sole close
   control in browse mode.

2. Mobile X collapses panel instead of dismissing file (bug): .mobile-close-btn
   was calling closeWorkspacePanel() directly, which collapsed the whole panel
   even when a file was open. Changed to handleWorkspaceClose(), which already
   has the correct two-step logic: clear preview first, close panel only if
   no preview is visible.

Files changed:
- static/boot.js: syncWorkspacePanelUI() hides btnClearPreview on desktop
  when hasPreview is false, guarded by !isCompact so mobile is unaffected
- static/index.html: mobile-close-btn onclick changed from
  closeWorkspacePanel() to handleWorkspaceClose()
- tests/test_sprint44.py: 10 new regression tests
- tests/test_mobile_layout.py: updated test_workspace_close_button_present()
  to accept handleWorkspaceClose() as the valid onclick target

* fix: widen test_server_delete_invalidates_index window to 1200 chars

The test extracted a 600-char window starting from the session/delete
handler to check for SESSION_INDEX_FILE. Commit 3cc5839 added session_id
character validation and path traversal guards before the unlink call,
pushing SESSION_INDEX_FILE to ~764 chars from the match — beyond the
600-char limit, causing the test to fail on CI.

Widened the window to 1200 chars, which accommodates any reasonable
amount of guard code before the SESSION_INDEX_FILE.unlink() call.

* docs: v0.50.33 release — version badge and CHANGELOG

---------

Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>

CHANGELOG.md

@@ -1,5 +1,20 @@
 # Hermes Web UI -- Changelog
 
+## [v0.50.33] fix: workspace panel close button — no duplicate X on desktop, mobile X respects file preview (#413)
+
+**Bug 1 — Duplicate X on desktop:** `#btnClearPreview` (the X icon) was always visible regardless of panel state, so desktop browse mode showed both the chevron collapse button and the X simultaneously. Fixed in `syncWorkspacePanelUI()`: on non-compact (desktop) viewports, `clearBtn.style.display` is set to `none` when no file preview is open, and cleared (shown) when a preview is active.
+
+**Bug 2 — Mobile X collapsed the whole panel instead of dismissing the file:** `.mobile-close-btn` was wired to `closeWorkspacePanel()` directly, bypassing the two-step close logic. Fixed by changing `onclick` to `handleWorkspaceClose()`, which calls `clearPreview()` first if a file is open, and falls through to `closeWorkspacePanel()` otherwise.
+
+**Also:** widened the `test_server_delete_invalidates_index` window from 600 → 1200 chars to accommodate the session_id validation guards added in v0.50.32 (#412).
+
+- `static/boot.js`: `syncWorkspacePanelUI()` sets `clearBtn.style.display` based on `hasPreview` when `!isCompact`
+- `static/index.html`: `.mobile-close-btn` onclick changed from `closeWorkspacePanel()` to `handleWorkspaceClose()`
+- `tests/test_sprint44.py`: 10 new regression tests covering both fixes
+- `tests/test_mobile_layout.py`: updated to accept `handleWorkspaceClose()` as valid onclick
+- `tests/test_regressions.py`: widened delete handler window to 1200 chars
+- 1051 tests total (up from 1041)
+
 ## [v0.50.32] fix(sessions): validate session_id before deleting session files [SECURITY] (#409)
 
 `/api/session/delete` accepted arbitrary `session_id` values from the request body and built the delete path directly as `SESSION_DIR / f"{sid}.json"`. Because pathlib discards the prefix when `sid` is an absolute path, an attacker could supply `/tmp/victim` and cause the server to unlink `victim.json` outside the session store. Traversal-style values (`../../etc/target`) were also accepted. CVSS 8.1 High (AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H).

static/boot.js

@@ -118,6 +118,10 @@ function syncWorkspacePanelUI(){
   if(clearBtn){
     clearBtn.disabled=!isOpen;
     clearBtn.title=hasPreview?'Close preview':'Hide workspace panel';
+    // On desktop, only show the X button when a file preview is open.
+    // In browse mode the chevron (btnCollapseWorkspacePanel) already serves
+    // as the close control, so showing both produces a duplicate X.
+    if(!isCompact) clearBtn.style.display=hasPreview?'':'none';
   }
 }
 

static/index.html

@@ -358,7 +358,7 @@
         <button class="panel-icon-btn" id="btnNewFolder" title="New folder" onclick="promptNewFolder()"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M22 19a2 2 0 0 1-2 2H4a2 2 0 0 1-2-2V5a2 2 0 0 1 2-2h5l2 3h9a2 2 0 0 1 2 2z"/></svg></button>
         <button class="panel-icon-btn" id="btnRefreshPanel" title="Refresh" onclick="if(S.session)loadDir(S.currentDir)"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><polyline points="23 4 23 10 17 10"/><polyline points="1 20 1 14 7 14"/><path d="M3.51 9a9 9 0 0 1 14.85-3.36L23 10M1 14l4.64 4.36A9 9 0 0 0 20.49 15"/></svg></button>
         <button class="panel-icon-btn close-preview" id="btnClearPreview" title="Close preview"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><line x1="18" y1="6" x2="6" y2="18"/><line x1="6" y1="6" x2="18" y2="18"/></svg></button>
-        <button class="panel-icon-btn mobile-close-btn" onclick="closeWorkspacePanel()" title="Close" aria-label="Close workspace panel">×</button>
+        <button class="panel-icon-btn mobile-close-btn" onclick="handleWorkspaceClose()" title="Close" aria-label="Close workspace panel">×</button>
       </div>
     </div>
     <div class="breadcrumb-bar" id="breadcrumbBar" style="display:none"></div>
@@ -535,7 +535,7 @@
                 <div class="settings-section-title">System</div>
                 <div class="settings-section-meta">Instance version and access controls.</div>
               </div>
-              <span class="settings-version-badge">v0.50.32</span>
+              <span class="settings-version-badge">v0.50.33</span>
             </div>
             <div class="settings-field" style="border-top:1px solid var(--border);padding-top:12px;margin-top:8px">
               <label for="settingsPassword" data-i18n="settings_label_password">Access Password</label>

tests/test_mobile_layout.py

@@ -115,13 +115,16 @@ def test_topbar_chips_mobile_overflow():
 
 def test_workspace_close_button_present():
     """Workspace panel must have a close/hide button accessible on mobile."""
-    # Either a dedicated mobile close button or the toggle button that closes the panel
+    # Accept handleWorkspaceClose() (two-step close: file→browse→closed), or the
+    # lower-level functions directly.  handleWorkspaceClose is preferred because
+    # it dismisses a file preview first before closing the panel.
     has_close = (
+        'onclick="handleWorkspaceClose()"' in HTML or
         'onclick="closeWorkspacePanel()"' in HTML or
         'onclick="toggleWorkspacePanel()"' in HTML
     )
     assert has_close, \
-        "closeWorkspacePanel() or toggleWorkspacePanel() must be wired to a button to close the workspace panel on mobile"
+        "handleWorkspaceClose() or closeWorkspacePanel() must be wired to a button to close the workspace panel on mobile"
 
 
 def test_toggle_mobile_files_js_defined():

tests/test_regressions.py

@@ -310,7 +310,10 @@ def test_server_delete_invalidates_index(cleanup_test_sessions):
             text.find('if parsed.path == "/api/session/delete":'),
         )
         if delete_idx >= 0:
-            delete_block = text[delete_idx:delete_idx+600]
+            # Use 1200 chars to accommodate any validation/guard code added
+            # before the SESSION_INDEX_FILE.unlink() call (e.g. session_id
+            # character checks, path traversal guards).
+            delete_block = text[delete_idx:delete_idx+1200]
             assert "SESSION_INDEX_FILE" in delete_block, \
                 f"{label} session/delete must invalidate SESSION_INDEX_FILE"
             return

tests/test_sprint44.py

@@ -0,0 +1,134 @@
+"""
+Sprint 44 Tests: Workspace panel close button fixes (PR #413).
+
+Covers:
+- index.html: mobile-close-btn now calls handleWorkspaceClose() instead of
+  closeWorkspacePanel(), so hitting X while a file is open returns you to the
+  file browser rather than collapsing the whole panel.
+- boot.js: syncWorkspacePanelUI() hides #btnClearPreview (the X icon) on
+  desktop when no file preview is open, eliminating the duplicate X that
+  appeared alongside the chevron collapse button.
+- boot.js: handleWorkspaceClose() logic — clears preview when one is visible,
+  closes panel otherwise (existing function, confirmed wired to both buttons).
+"""
+import pathlib
+import re
+import unittest
+
+REPO = pathlib.Path(__file__).parent.parent
+HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8")
+BOOT_JS = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
+
+
+class TestMobileCloseButtonBehavior(unittest.TestCase):
+    """mobile-close-btn must call handleWorkspaceClose(), not closeWorkspacePanel()."""
+
+    def test_mobile_close_btn_calls_handle_workspace_close(self):
+        """mobile-close-btn onclick must be handleWorkspaceClose(), not closeWorkspacePanel()."""
+        m = re.search(r'class="[^"]*mobile-close-btn[^"]*"[^>]*>', HTML)
+        self.assertIsNotNone(m, "mobile-close-btn element not found in index.html")
+        btn_html = m.group(0)
+        self.assertIn(
+            'onclick="handleWorkspaceClose()"',
+            btn_html,
+            "mobile-close-btn must call handleWorkspaceClose() so that hitting X "
+            "while a file is open closes the file first, not the whole panel",
+        )
+
+    def test_mobile_close_btn_does_not_call_close_workspace_panel_directly(self):
+        """mobile-close-btn must NOT call closeWorkspacePanel() directly."""
+        m = re.search(r'class="[^"]*mobile-close-btn[^"]*"[^>]*>', HTML)
+        self.assertIsNotNone(m, "mobile-close-btn element not found in index.html")
+        btn_html = m.group(0)
+        self.assertNotIn(
+            'onclick="closeWorkspacePanel()"',
+            btn_html,
+            "mobile-close-btn must not call closeWorkspacePanel() directly — "
+            "it would bypass the two-step close logic and collapse the panel even "
+            "when a file is being viewed",
+        )
+
+    def test_handle_workspace_close_defined_in_boot_js(self):
+        """handleWorkspaceClose() must be defined in boot.js."""
+        self.assertIn(
+            "function handleWorkspaceClose()",
+            BOOT_JS,
+            "handleWorkspaceClose() is missing from boot.js",
+        )
+
+    def test_handle_workspace_close_clears_preview_first(self):
+        """handleWorkspaceClose() must call clearPreview() when a preview is visible."""
+        # The function must check for visible preview and call clearPreview
+        self.assertIn(
+            "clearPreview()",
+            BOOT_JS,
+            "handleWorkspaceClose() must call clearPreview() when preview is visible",
+        )
+    def test_handle_workspace_close_falls_back_to_close_panel(self):
+        """handleWorkspaceClose() must call closeWorkspacePanel() as fallback."""
+        # Find the function start and extract until the closing brace by scanning
+        start = BOOT_JS.find("function handleWorkspaceClose()")
+        self.assertNotEqual(start, -1, "handleWorkspaceClose() not found in boot.js")
+        # Extract a generous window after the function start
+        fn_window = BOOT_JS[start : start + 400]
+        self.assertIn(
+            "closeWorkspacePanel()",
+            fn_window,
+            "handleWorkspaceClose() must call closeWorkspacePanel() as its fallback path",
+        )
+
+
+class TestDesktopNoDuplicateXButton(unittest.TestCase):
+    """On desktop, only one X/close control should appear at a time."""
+
+    def test_sync_workspace_panel_ui_hides_clear_preview_on_desktop(self):
+        """syncWorkspacePanelUI() must set display:none on btnClearPreview when no preview and desktop."""
+        self.assertIn(
+            "clearBtn.style.display",
+            BOOT_JS,
+            "syncWorkspacePanelUI() must control clearBtn.style.display to hide it "
+            "on desktop when no file preview is open",
+        )
+
+    def test_clear_preview_hidden_when_no_preview(self):
+        """The display toggle for btnClearPreview must key off hasPreview."""
+        # Expect something like: clearBtn.style.display=hasPreview?'':'none'
+        # or clearBtn.style.display = hasPreview ? '' : 'none'
+        pattern = r"clearBtn\.style\.display\s*=\s*hasPreview"
+        self.assertRegex(
+            BOOT_JS,
+            pattern,
+            "btnClearPreview display must be conditioned on hasPreview in "
+            "syncWorkspacePanelUI() to avoid a duplicate X on desktop",
+        )
+
+    def test_clear_preview_toggle_only_applied_on_desktop(self):
+        """The display toggle must be guarded by !isCompact so mobile is unaffected."""
+        # Expect: if(!isCompact) clearBtn.style.display=...
+        pattern = r"isCompact.*clearBtn\.style\.display|clearBtn\.style\.display.*isCompact"
+        self.assertRegex(
+            BOOT_JS,
+            pattern,
+            "btnClearPreview display toggle must be guarded by isCompact so the "
+            "mobile X button visibility is not accidentally affected",
+        )
+
+    def test_btnclearpreview_exists_in_html(self):
+        """#btnClearPreview must still exist in the HTML (not removed)."""
+        self.assertIn(
+            'id="btnClearPreview"',
+            HTML,
+            "#btnClearPreview must remain in index.html",
+        )
+
+    def test_btncollapseWorkspacepanel_exists_in_html(self):
+        """#btnCollapseWorkspacePanel (chevron) must still exist in the HTML."""
+        self.assertIn(
+            'id="btnCollapseWorkspacePanel"',
+            HTML,
+            "#btnCollapseWorkspacePanel must remain in index.html",
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()