fix(tools): parse Hermes/Qwen JSON bodies inside tool_call wrappers (#5887)

parse_tool_blocks fed <tool_call> wrapper bodies only to the XML
iterators (_iter_xml_invoke/_iter_xml_direct), so the canonical
Qwen/Hermes text-mode form — a bare JSON object like
{"name": "bash", "arguments": {"command": "..."}} inside the
wrapper — parsed to zero tool blocks and the agent never executed
anything. Pattern 4d only matches OpenAI-style blobs with a literal
"function" key, which the Hermes format lacks.

Wrapper bodies are now classified first: a JSON-looking body ({ or [)
is parsed by the new _parse_json_tool_call_body, which requires an
object with a string "name" and rejects a non-object "arguments"
instead of coercing it, then converts through the same
function_call_to_tool_block used by the XML paths so aliases and
per-tool argument formatting stay uniform. JSON-looking bodies fail
closed — they are never rescanned by the XML iterators (including the
unclosed-wrapper and bare-invoke fallbacks), so XML-like text inside
JSON argument values stays data instead of selecting a different tool.
Non-JSON bodies keep the existing XML path unchanged.

Fixes #5187

Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>

routes/email_routes.py

@@ -5954,7 +5954,7 @@ def setup_email_routes():
             raise HTTPException(400, "GOOGLE_OAUTH_CLIENT_ID not set — add it to .env")
         redirect_uri = (
             os.environ.get("GOOGLE_OAUTH_REDIRECT_URI")
-            or f"http://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
+            or f"{request.url.scheme}://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
         )
         state = make_oauth_state(account_id, owner)
         params = urllib.parse.urlencode({
@@ -5991,7 +5991,7 @@ def setup_email_routes():
         client_secret = os.environ.get("GOOGLE_OAUTH_CLIENT_SECRET", "")
         redirect_uri = (
             os.environ.get("GOOGLE_OAUTH_REDIRECT_URI")
-            or f"http://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
+            or f"{request.url.scheme}://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback"
         )
         import httpx as _httpx
         try:

routes/gallery/gallery_routes.py

@@ -127,6 +127,25 @@ def _load_grounding_backend():
     return cached
 
 
+def _model_input_to_device(value, device: str, torch):
+    if not hasattr(value, "to"):
+        return value
+    if (
+        device == "mps"
+        and hasattr(torch, "float64")
+        and getattr(value, "dtype", None) == torch.float64
+    ):
+        return value.to(device=device, dtype=torch.float32)
+    return value.to(device)
+
+
+def _model_inputs_to_device(inputs, device: str, torch) -> Dict[str, Any]:
+    return {
+        key: _model_input_to_device(value, device, torch)
+        for key, value in inputs.items()
+    }
+
+
 def _ground_text_to_box(image, text: str, *, threshold: float = 0.05):
     query = (text or "").strip()
     if not query:
@@ -142,10 +161,7 @@ def _ground_text_to_box(image, text: str, *, threshold: float = 0.05):
         labels.append(f"a photo of {query}")
     try:
         inputs = processor(text=[labels], images=image, return_tensors="pt")
-        model_inputs = {
-            k: (v.to(device) if hasattr(v, "to") else v)
-            for k, v in inputs.items()
-        }
+        model_inputs = _model_inputs_to_device(inputs, device, torch)
         with torch.no_grad():
             outputs = model(**model_inputs)
         target_sizes = torch.tensor([[image.height, image.width]])
@@ -1869,10 +1885,7 @@ def setup_gallery_routes() -> APIRouter:
 
         try:
             inputs = processor(image, **kwargs)
-            model_inputs = {
-                k: (v.to(device) if hasattr(v, "to") else v)
-                for k, v in inputs.items()
-            }
+            model_inputs = _model_inputs_to_device(inputs, device, torch)
             with torch.no_grad():
                 outputs = model(**model_inputs)
             masks = processor.image_processor.post_process_masks(

services/memory/skill_format.py

@@ -100,6 +100,18 @@ def _parse_scalar(raw: str) -> Any:
     if raw.lower() in ("null", "none", "~"):
         return None
     if (raw[0] == raw[-1]) and raw[0] in ("'", '"'):
+        if raw[0] == '"':
+            # _emit_scalar writes double-quoted scalars with json.dumps, so
+            # decode the escapes instead of only stripping the quotes. Without
+            # this, `\"` / `\\` / `\uXXXX` stayed verbatim in the value and the
+            # next save escaped their backslashes again, doubling them on every
+            # load/save cycle (issue #5210).
+            try:
+                return json.loads(raw)
+            except ValueError:
+                # Hand-written file using escapes JSON rejects (e.g. a bare
+                # Windows path). Keep the previous literal reading.
+                pass
         return raw[1:-1]
     # Try number
     try:
@@ -171,6 +183,26 @@ def parse_frontmatter(text: str) -> tuple[Dict[str, Any], str]:
     return fm, body
 
 
+# Characters that force a quoted scalar. The punctuation would otherwise change
+# how the value reads back; the second row is every character str.splitlines()
+# treats as a line break, and parse_frontmatter() reads one scalar per line, so
+# emitting one of those bare would split the value across lines.
+_FM_MUST_QUOTE = (
+    ":", "#", "[", "]", "{", "}", ",", "&", "*", "!", "|", ">", "'", '"', "%", "@",
+    "\n", "\r", "\v", "\f", "\x1c", "\x1d", "\x1e", "\x85", "\u2028", "\u2029",
+)
+
+# json.dumps escapes every C0 control character, but with ensure_ascii=False it
+# passes NEL / LINE SEPARATOR / PARAGRAPH SEPARATOR through literally, and
+# str.splitlines() still breaks on all three. Re-escape exactly those, which
+# json.loads decodes again on the way in, so the pair stays symmetric.
+_FM_POST_DUMPS_ESCAPES = (
+    ("\x85", "\\u0085"),
+    ("\u2028", "\\u2028"),
+    ("\u2029", "\\u2029"),
+)
+
+
 def _emit_scalar(v: Any) -> str:
     if v is None:
         return "null"
@@ -181,8 +213,15 @@ def _emit_scalar(v: Any) -> str:
     if isinstance(v, list):
         return "[" + ", ".join(_emit_scalar(x) for x in v) + "]"
     s = str(v)
-    if any(c in s for c in (":", "#", "\n", "[", "]", "{", "}", ",", "&", "*", "!", "|", ">", "'", '"', "%", "@")):
-        return json.dumps(s)
+    if any(c in s for c in _FM_MUST_QUOTE):
+        # ensure_ascii=False keeps non-ASCII text as itself. SKILL.md is UTF-8 at
+        # both ends (skills.py reads it, atomic_write_text writes it), so the
+        # \uXXXX form bought nothing and leaked into the parsed value (#5210).
+        out = json.dumps(s, ensure_ascii=False)
+        for ch, esc in _FM_POST_DUMPS_ESCAPES:
+            if ch in out:
+                out = out.replace(ch, esc)
+        return out
     return s
 
 

src/llm_core.py

@@ -1,6 +1,7 @@
 # src/llm_core.py
 import httpx
 import asyncio
+import copy
 import time
 import json
 import logging
@@ -644,7 +645,7 @@ def _build_ollama_payload(
     if options:
         payload["options"] = options
     if tools:
-        payload["tools"] = tools
+        payload["tools"] = _alias_harmony_tools(tools, model)
     return payload
 
 
@@ -1055,6 +1056,57 @@ def _model_disallows_reasoning_effort_with_chat_tools(model: str) -> bool:
     return bool(re.match(r"^(?:openai/)?gpt-5(?:[.\-]\d+)?(?:[-_:].*)?$", m))
 
 
+# gpt-oss (harmony) ships BUILT-IN tools named `python` and `browser`, invoked
+# with the raw body as the argument (`to=python` + bare source), while custom
+# functions use `to=functions.NAME` + JSON. A tool we expose under a built-in's
+# name therefore gets called with the built-in convention: the model emits raw
+# code, the server tries to parse it as JSON, and the whole request dies
+# ("error parsing tool call: raw='import sys, ...'"). In streaming mode Ollama
+# does not even report it — it truncates the stream, so the turn looks like an
+# empty response. `bash` collides the same way in practice.
+#
+# Measured on gpt-oss:20b via Ollama /v1 with a fixed agentic prompt:
+#   tools named python+bash ............ 2/6 succeeded (4 parse failures)
+#   python renamed ..................... 5/6
+#   python and bash renamed ............ 6/6
+#
+# So rename the colliding tools on the way out and map the names back on the
+# way in. Confined to the transport layer: callers keep using the real names.
+_HARMONY_TOOL_ALIASES = {
+    "python": "run_python_code",
+    "bash": "run_shell_command",
+    "browser": "web_browser_tool",
+}
+_HARMONY_TOOL_ALIASES_REVERSE = {v: k for k, v in _HARMONY_TOOL_ALIASES.items()}
+
+
+def _is_harmony_model(model: str) -> bool:
+    """True for gpt-oss / harmony-format models, which have built-in tool names."""
+    return "gpt-oss" in (model or "").lower()
+
+
+def _alias_harmony_tools(tools: Optional[List[Dict]], model: str) -> Optional[List[Dict]]:
+    """Rename tools that collide with harmony built-ins. Returns a copy."""
+    if not tools or not _is_harmony_model(model):
+        return tools
+    out = []
+    for t in tools:
+        fn = t.get("function") or {}
+        alias = _HARMONY_TOOL_ALIASES.get(fn.get("name"))
+        if alias:
+            t = copy.deepcopy(t)
+            t["function"]["name"] = alias
+        out.append(t)
+    return out
+
+
+def _unalias_harmony_tool_name(name: str, model: str) -> str:
+    """Map an aliased tool name in a model response back to the real name."""
+    if not _is_harmony_model(model):
+        return name
+    return _HARMONY_TOOL_ALIASES_REVERSE.get(name, name)
+
+
 def _scrub_openai_chat_tool_reasoning(payload: Dict, target_url: str, model: str) -> None:
     if not payload.get("tools"):
         return
@@ -2109,7 +2161,17 @@ async def llm_call_async(
                     response = _parse_ollama_response(data)
                 else:
                     msg = data["choices"][0]["message"]
-                    response = msg.get("content") or msg.get("reasoning_content") or ""
+                    content = msg.get("content")
+                    if isinstance(content, list):
+                        # Mistral structured content — extract thinking + text
+                        # (same contract as llm_call / stream_llm; see #5435).
+                        text_part, thinking_part = _normalize_mistral_content(content)
+                        if thinking_part:
+                            response = thinking_part + "\n\n" + (text_part or "")
+                        else:
+                            response = text_part or msg.get("reasoning_content") or ""
+                    else:
+                        response = content or msg.get("reasoning_content") or ""
                 _set_cached_response(cache_key, response)
                 return response
             except Exception:
@@ -2226,7 +2288,7 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
             tok_key = "max_completion_tokens" if _uses_max_completion_tokens(model) else "max_tokens"
             payload[tok_key] = max_tokens
         if tools:
-            payload["tools"] = tools
+            payload["tools"] = _alias_harmony_tools(tools, model)
         elif tool_choice_none:
             payload["tool_choice"] = "none"
         # Mistral thinking-capable models — send reasoning_effort so Mistral
@@ -2360,7 +2422,7 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
                         if fn.get("name"):
                             _ollama_tool_calls.append({
                                 "id": tc.get("id") or f"call_{len(_ollama_tool_calls)}",
-                                "name": fn.get("name") or "",
+                                "name": _unalias_harmony_tool_name(fn.get("name") or "", model),
                                 "arguments": json.dumps(fn.get("arguments") or {}),
                             })
                     if j.get("done"):
@@ -2740,7 +2802,10 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
                                             if tc.get("extra_content"):
                                                 _tc_acc[idx]["extra_content"] = tc["extra_content"]
                                             if func.get("name"):
-                                                _tc_acc[idx]["name"] = func["name"]
+                                                # Map harmony aliases back to real
+                                                # tool names before anything
+                                                # downstream sees them.
+                                                _tc_acc[idx]["name"] = _unalias_harmony_tool_name(func["name"], model)
                                             if "arguments" in func:
                                                 # Guard against a null arguments delta: `func` can be
                                                 # {"arguments": None} (JSON null), and a raw `+= None`

src/tool_parsing.py

@@ -193,7 +193,7 @@ _QWEN_ROLE_MARKER_RE = re.compile(r"</?\|(?:assistant|assistan|user|system|tool)
 # one; see #5547. `|end`, `end|`, `|end|` and `/|end|` still strip as before.
 _QWEN_BARE_MARKER_RE = re.compile(
     r"(?:^|[\t\r\n ])(?:/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|"
-    r"(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)",
+    r"(?:^|[\r\n])[ \t]*assistan(?:t)?[ \t]*(?=[\r\n]|$)",
     re.IGNORECASE,
 )
 
@@ -929,6 +929,46 @@ def _parse_xml_direct_tool(name, body) -> Optional[ToolBlock]:
     return function_call_to_tool_block(mapped, json.dumps(params))
 
 
+def _looks_like_json_body(body: str) -> bool:
+    """True when a <tool_call> wrapper body is JSON, not XML markup."""
+    return body.lstrip()[:1] in ("{", "[")
+
+
+def _parse_json_tool_call_body(body: str) -> Optional[ToolBlock]:
+    """Parse a Qwen/Hermes text-mode wrapper body: bare JSON inside <tool_call>.
+
+      <tool_call>
+      {"name": "bash", "arguments": {"command": "mkdir -p agent-test"}}
+      </tool_call>
+
+    Strict by design (issue #5187 / tracker #5333): the body must decode to an
+    object with a string "name", and "arguments" — when present — must itself
+    be an object. Anything else returns None rather than being coerced, so a
+    malformed call is dropped instead of dispatching with mangled arguments.
+    raw_decode tolerates trailing chatter after the JSON object; the trailing
+    text is never scanned for tool markup. Conversion goes through
+    function_call_to_tool_block so aliases and per-tool argument formatting
+    stay identical to the XML invoke path.
+    """
+    stripped = body.strip()
+    if not stripped.startswith("{"):
+        return None
+    try:
+        parsed, _end = json.JSONDecoder().raw_decode(stripped)
+    except json.JSONDecodeError:
+        return None
+    if not isinstance(parsed, dict):
+        return None
+    name = parsed.get("name")
+    if not isinstance(name, str) or not name.strip():
+        return None
+    if "arguments" in parsed and not isinstance(parsed["arguments"], dict):
+        return None
+    args = parsed.get("arguments", {})
+    from src.tool_schemas import function_call_to_tool_block
+    return function_call_to_tool_block(name.strip().lower(), json.dumps(args))
+
+
 def _iter_stepfun_tool_calls(text: str):
     """Yield StepFun native tool-call token bodies without regex backtracking."""
     pos = 0
@@ -1330,10 +1370,21 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
         if blocks:
             return blocks
         # Try wrapped: <tool_call><invoke ...>...</invoke></tool_call>
+        # A wrapper body that is JSON (Qwen/Hermes text mode, issue #5187) is
+        # parsed as JSON or dropped — never scanned by the XML iterators, so
+        # XML-like text inside JSON argument values stays data instead of
+        # selecting a different tool.
+        json_body_seen = False
         for _ms, inner_start, inner_end, _me in _iter_delimited(
             text, _XML_TOOL_CALL_OPEN_RE, _XML_TOOL_CALL_CLOSE_RE
         ):
             body = text[inner_start:inner_end]
+            if _looks_like_json_body(body):
+                json_body_seen = True
+                block = _parse_json_tool_call_body(body)
+                if block:
+                    blocks.append(block)
+                continue
             for inv_name, inv_body in _iter_xml_invoke(body):
                 block = _parse_xml_invoke(inv_name, inv_body)
                 if block:
@@ -1348,6 +1399,13 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
         if not blocks:
             for m in _XML_OPEN_TOOL_CALL_RE.finditer(text):
                 body = m.group(1)
+                if _looks_like_json_body(body):
+                    # Same fail-closed rule as above for an unclosed wrapper.
+                    json_body_seen = True
+                    block = _parse_json_tool_call_body(body)
+                    if block:
+                        blocks.append(block)
+                    break
                 for inv_name, inv_body in _iter_xml_invoke(body):
                     block = _parse_xml_invoke(inv_name, inv_body)
                     if block:
@@ -1358,8 +1416,11 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
                     block = _parse_xml_direct_tool(d_name, d_body)
                     if block:
                         blocks.append(block)
-        # Try bare <invoke> without wrapper
-        if not blocks:
+        # Try bare <invoke> without wrapper. Skipped when a JSON wrapper body
+        # was seen but produced no block: this rescan covers the full text,
+        # wrapper bodies included, and <invoke> markup inside a (possibly
+        # malformed) JSON payload must stay data rather than dispatch.
+        if not blocks and not json_body_seen:
             for inv_name, inv_body in _iter_xml_invoke(text):
                 block = _parse_xml_invoke(inv_name, inv_body)
                 if block:

static/app.js

@@ -28,6 +28,7 @@ import memoryModule from './js/memory.js?v=20260722memoryloading1';
 import voiceRecorderModule from './js/voiceRecorder.js';
 import censorModule from './js/censor.js';
 import galleryModule from './js/gallery.js';
+import { UI_VIS_DEFAULT_OFF, resolveVisibility } from './js/ui_visibility.js';
 import tasksModule from './js/tasks.js?v=20260723tasksbulkfeedback1';
 import calendarModule from './js/calendar.js';
 import notesModule from './js/notes.js';
@@ -2725,46 +2726,6 @@ function initializeEventListeners() {
   // ── UI Visibility (Customize UI modal) ──
   const UI_VIS_KEY = 'odysseus-ui-visibility';
 
-  // Selector map: key → CSS selector(s) for targets
-  const UI_VIS_MAP = {
-    'sidebar-brand':       '.sidebar-brand-title',
-    'sidebar-new-chat':    '#sidebar-new-chat-btn',
-    'sidebar-search':      '#sidebar-search-btn',
-	    'sessions-section':    '#sessions-section',
-	    'email-section':       '#email-section',
-	    'tools-section':       '#tools-section',
-    // Per-tool visibility — fine-grained control over which entries show
-    // inside the Tools section in the sidebar.
-    'tool-calendar':       '#tool-calendar-btn',
-    'tool-compare':        '#tool-compare-btn',
-    'tool-cookbook':       '#tool-cookbook-btn',
-    'tool-research':       '#tool-research-btn',
-    'tool-gallery':        '#tool-gallery-btn',
-    'tool-library':        '#tool-library-btn',
-    'tool-memory':         '#tool-memory-btn',
-    'tool-notes':          '#tool-notes-btn',
-    'tool-tasks':          '#tool-tasks-btn',
-    'tool-theme':          '#tool-theme-btn',
-    'user-bar':            '#user-bar-profile',
-    'sidebar-settings-btn':'#user-bar-settings',
-    'chat-meta':           '.chat-meta-overlay',
-    'welcome-text':        '.welcome-name, .welcome-sub, #welcome-tip',
-    'incognito-btn':       '.incognito-btn',
-    'web-toggle-btn':      '#web-toggle-btn',
-    'doc-toggle-btn':      '#overflow-doc-btn',
-    'rag-toggle-btn':      '#overflow-rag-btn',
-    'bash-toggle-btn':     '#bash-toggle-btn',
-    'overflow-plus-btn':   '.overflow-wrapper',
-    'mode-toggle':         '.mode-toggle',
-    'preset-mini-btn':     '#overflow-preset-btn',
-    'attach-btn':          '#overflow-attach-btn',
-    'research-btn':        '#overflow-research-btn',
-    'rail-new-chat':       '#rail-new-session',
-  };
-
-  // Keys hidden by default on first run (no localStorage yet)
-	  const UI_VIS_DEFAULT_OFF = new Set(['rag-toggle-btn', 'text-emojis', 'chat-fullwidth']);
-
   // Keys that need admin to toggle off (reserved for future use)
   const UI_VIS_ADMIN_ONLY = new Set([]);
 
@@ -2777,14 +2738,14 @@ function initializeEventListeners() {
   }
 
   function applyUIVis(state) {
-    Object.entries(UI_VIS_MAP).forEach(([key, selector]) => {
-      // section-drag-reorder uses a body class instead of inline styles
-      if (key === 'section-drag-reorder') return;
-      const visible = key in state ? state[key] !== false : !UI_VIS_DEFAULT_OFF.has(key);
+    // resolveVisibility computes selector→visible (pure; ui_visibility.js),
+    // including the tools-section parent rule that hides every tool rail
+    // launcher when Tools is off. Apply the result to the DOM here.
+    for (const [selector, visible] of Object.entries(resolveVisibility(state))) {
       document.querySelectorAll(selector).forEach(el => {
         el.style.display = visible ? '' : 'none';
       });
-    });
+    }
     // Drag reorder: use body class so dynamically created handles are covered
     const dragEnabled = state['section-drag-reorder'] === true;
     document.body.classList.toggle('rearrange-mode', dragEnabled);

static/index.html

@@ -248,6 +248,15 @@
       }, { once: true });
     })();
   </script>
+  <!-- Preload the two faces first paint actually uses: Fira Code 400 and 600,
+       the app font and the weight the sidebar and header text render at. They
+       are declared in style.css, so without a hint they are only discovered
+       after the stylesheet parses and then queue behind the module graph.
+       crossorigin is required even though these are same-origin: fonts are
+       always fetched in CORS mode, and a preload whose mode does not match the
+       real request is discarded and the font fetched a second time. -->
+  <link rel="preload" as="font" type="font/woff2" crossorigin href="/static/fonts/FiraCode-Regular.woff2">
+  <link rel="preload" as="font" type="font/woff2" crossorigin href="/static/fonts/FiraCode-SemiBold.woff2">
   <link rel="stylesheet" href="/static/style.css?v=20260808startupshell1">
   <link rel="modulepreload" href="/static/app.js?v=20260808startupshell1">
   <link rel="modulepreload" href="/static/js/chat.js?v=20260801fix1">

static/js/chatRenderer.js

@@ -481,7 +481,7 @@ const QWEN_ROLE_MARKER_RE = /<\/?\|(?:assistant|assistan|user|system|tool)\|>?|<
 // Keep in sync with _QWEN_BARE_MARKER_RE in src/tool_parsing.py. At least one
 // pipe is required around `end`: with both optional (`\|?end\|?`) this also ate
 // a bare `end` on its own line, breaking Ruby/Lua/shell snippets (#5547).
-const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)/gi;
+const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|(?:^|[\r\n])[ \t]*assistan(?:t)?[ \t]*(?=[\r\n]|$)/gi;
 // Self-narration about tool results (model echoing stdout/exit_code)
 const TOOL_NARRATION_RE = /(?:The (?:result|output) shows?:?\s*)?-?\s*(?:stdout|stderr|exit_code):\s*.+/gi;
 

static/js/spinner.js

@@ -4,6 +4,13 @@
  * ASCII Spinner Module for AI thinking/processing status
  */
 
+// How long a canvas spinner may keep animating before its element has ever
+// been inserted into the document. start() runs synchronously, before the
+// caller appends the element, so frame 1 is always disconnected. Callers do
+// append in the same task, so anything past this window means the element is
+// never coming and the frames are drawing for nobody.
+const UNATTACHED_GRACE_MS = 2000;
+
 class Spinner {
   constructor(message = "AI is processing", style = "right", animation = "spinner") {
     // Different animation frames
@@ -21,6 +28,9 @@ class Spinner {
     this.intervalId = null;
     this.rafId = null;
     this.element = null;
+    this._wpWasConnected = false;
+    this._wpUnattachedSince = null;
+    this._visHandler = null;
   }
 
   /**
@@ -74,6 +84,7 @@ class Spinner {
   }
 
   _drawSineWave() {
+    if (!this.isRunning) return;
     const ctx = this._ctx;
     const W = this._canvas.width;
     const H = this._canvas.height;
@@ -120,9 +131,7 @@ class Spinner {
     ctx.fillStyle = 'rgba(156, 222, 242, 0.9)';
     ctx.fill();
 
-    if (this.isRunning) {
-      this.rafId = requestAnimationFrame(() => this._drawSineWave());
-    }
+    if (this.isRunning) this._requestFrame();
   }
 
   _createWhirlpoolElement() {
@@ -158,6 +167,7 @@ class Spinner {
   }
 
   _drawWhirlpool() {
+    if (!this.isRunning) return;
     const ctx = this._wpCtx;
     const W = this._wpCanvas.width;
     const H = this._wpCanvas.height;
@@ -229,18 +239,77 @@ class Spinner {
     ctx.fill();
     ctx.globalAlpha = 1;
 
-    if (!this.isRunning) return;
-    // Leak-safe self-terminate: stop once our element WAS in the DOM and then
-    // got removed (e.g. a loading row replaced by results). But keep spinning
-    // before it's first appended — start() runs synchronously, before the
-    // caller inserts the element, so it isn't connected on frame 1.
+    // Leak-safe self-terminate. "Nobody can see this spinner" has two shapes
+    // and we have to catch both:
+    //   1. the element WAS in the DOM and then got removed (a loading row
+    //      replaced by results);
+    //   2. the element was NEVER inserted, and the grace window for inserting
+    //      it has expired. The caller started a spinner and then took an early
+    //      return (aborted request, panel that resolved from cache), so no
+    //      frame we draw will ever be observed.
+    // Case 2 is why this needs a deadline at all: while the element has never
+    // been connected, `!this._wpWasConnected` stays true forever, so without
+    // the grace check the loop re-arms until the tab closes.
     const connected = !!(this.element && this.element.isConnected);
-    if (connected) this._wpWasConnected = true;
-    if (connected || !this._wpWasConnected) {
-      this.rafId = requestAnimationFrame(() => this._drawWhirlpool());
-    } else {
-      this.isRunning = false;
+    if (connected) {
+      this._wpWasConnected = true;
+      this._wpUnattachedSince = null;
+    } else if (!this._wpWasConnected) {
+      if (this._wpUnattachedSince === null) this._wpUnattachedSince = performance.now();
+      if (performance.now() - this._wpUnattachedSince > UNATTACHED_GRACE_MS) {
+        this.stop();
+        return;
+      }
     }
+
+    if (connected || !this._wpWasConnected) {
+      this._requestFrame();
+    } else {
+      this.stop();
+    }
+  }
+
+  /**
+   * Arm the next animation frame. Clearing rafId as the callback enters keeps
+   * it a truthful "a frame is pending" flag, which is what stop() and the
+   * visibility handler cancel against.
+   */
+  _requestFrame() {
+    this.rafId = requestAnimationFrame(() => {
+      this.rafId = null;
+      if (this.animation === 'sinewave') this._drawSineWave();
+      else this._drawWhirlpool();
+    });
+  }
+
+  /**
+   * Stop drawing while the tab is hidden. Browsers throttle background rAF but
+   * do not reliably stop the canvas work, and a spinner nobody is looking at
+   * should cost nothing. The listener is owned by start()/stop() so it is never
+   * left behind on a dead spinner.
+   */
+  _armVisibilityPause() {
+    if (this._visHandler) return;
+    this._visHandler = () => {
+      if (document.hidden) {
+        if (this.rafId) {
+          cancelAnimationFrame(this.rafId);
+          this.rafId = null;
+        }
+      } else if (this.isRunning && !this.rafId) {
+        // Reset the wave clock so the hidden interval doesn't arrive as one
+        // huge dt and skip the animation forward.
+        this._wavePrev = performance.now();
+        this._requestFrame();
+      }
+    };
+    document.addEventListener('visibilitychange', this._visHandler);
+  }
+
+  _disarmVisibilityPause() {
+    if (!this._visHandler) return;
+    document.removeEventListener('visibilitychange', this._visHandler);
+    this._visHandler = null;
   }
 
   /**
@@ -272,12 +341,15 @@ class Spinner {
 
     if (this.animation === 'sinewave') {
       this._wavePrev = performance.now();
+      this._armVisibilityPause();
       this._drawSineWave();
       return;
     }
 
     if (this.animation === 'whirlpool') {
       this._wpStartedAt = performance.now();
+      this._wpUnattachedSince = null;
+      this._armVisibilityPause();
       this._drawWhirlpool();
       return;
     }
@@ -302,6 +374,7 @@ class Spinner {
       cancelAnimationFrame(this.rafId);
       this.rafId = null;
     }
+    this._disarmVisibilityPause();
   }
 
   /**

static/js/ui_visibility.js

@@ -0,0 +1,73 @@
+// static/js/ui_visibility.js
+//
+// Per-item visibility for the sidebar and collapsed icon rail. Drives the
+// Settings → Appearance ("Customize UI") checkboxes, persisted in localStorage
+// under `odysseus-ui-visibility` (loaded/saved by app.js).
+//
+// Each key maps to the CSS selector(s) it controls. Tool/section selectors pair
+// the full-sidebar element with its #rail-* launcher so a tab hidden in the
+// full view also hides when the sidebar is minimized to the icon rail (id
+// mapping mirrors _railToolMap in app.js; #tool-library-btn ↔ #rail-archive).
+
+// Selector map: UI customization key → CSS selector(s) for its target(s).
+export const UI_VIS_MAP = {
+  'sidebar-brand':       '.sidebar-brand-title',
+  'sidebar-new-chat':    '#sidebar-new-chat-btn',
+  'sidebar-search':      '#sidebar-search-btn',
+  'sessions-section':   '#sessions-section',
+  'email-section':       '#email-section, #rail-email',
+  'tools-section':       '#tools-section',
+  // Per-tool entries pair the sidebar button with its rail launcher.
+  'tool-calendar':       '#tool-calendar-btn, #rail-calendar',
+  'tool-compare':        '#tool-compare-btn, #rail-compare',
+  'tool-cookbook':       '#tool-cookbook-btn, #rail-cookbook',
+  'tool-research':       '#tool-research-btn, #rail-research',
+  'tool-gallery':        '#tool-gallery-btn, #rail-gallery',
+  'tool-library':        '#tool-library-btn, #rail-archive',
+  'tool-memory':         '#tool-memory-btn, #rail-memory',
+  'tool-notes':          '#tool-notes-btn, #rail-notes',
+  'tool-tasks':          '#tool-tasks-btn, #rail-tasks',
+  'tool-theme':          '#tool-theme-btn, #rail-theme',
+  'user-bar':            '#user-bar-profile',
+  'sidebar-settings-btn':'#user-bar-settings',
+  'chat-meta':           '.chat-meta-overlay',
+  'welcome-text':        '.welcome-name, .welcome-sub, #welcome-tip',
+  'incognito-btn':       '.incognito-btn',
+  'web-toggle-btn':      '#web-toggle-btn',
+  'doc-toggle-btn':      '#overflow-doc-btn',
+  'rag-toggle-btn':      '#overflow-rag-btn',
+  'bash-toggle-btn':     '#bash-toggle-btn',
+  'overflow-plus-btn':   '.overflow-wrapper',
+  'mode-toggle':         '.mode-toggle',
+  'preset-mini-btn':     '#overflow-preset-btn',
+  'attach-btn':          '#overflow-attach-btn',
+  'research-btn':        '#overflow-research-btn',
+  'rail-new-chat':       '#rail-new-session',
+};
+
+// Keys hidden by default on first run (no localStorage yet).
+export const UI_VIS_DEFAULT_OFF = new Set(['rag-toggle-btn', 'text-emojis', 'chat-fullwidth']);
+
+/**
+ * Resolve every UI_VIS_MAP selector to visible (true) or hidden (false) for the
+ * given saved state. Pure: no DOM, no localStorage — app.js applies the result.
+ *
+ * A key is visible when its stored value is not `false`, defaulting to on
+ * unless it is in UI_VIS_DEFAULT_OFF. Per-tool entries also require the Tools
+ * section to be on: hiding Tools hides every tool, mirroring the full sidebar
+ * where the #tools-section container already hides them (the rail has no
+ * container, so this rule keeps it in sync).
+ *
+ * @param {Record<string, boolean>} state
+ * @returns {Record<string, boolean>} selector → visible
+ */
+export const resolveVisibility = (state = {}) => {
+  const toolsOn = state['tools-section'] !== false;
+  const out = {};
+  for (const [key, selector] of Object.entries(UI_VIS_MAP)) {
+    let visible = key in state ? state[key] !== false : !UI_VIS_DEFAULT_OFF.has(key);
+    if (!toolsOn && key.startsWith('tool-')) visible = false;
+    out[selector] = visible;
+  }
+  return out;
+};

tests/test_email_oauth.py

@@ -29,6 +29,7 @@ import base64
 import json
 import time
 import unittest.mock as mock
+from types import SimpleNamespace
 
 import pytest
 
@@ -272,8 +273,14 @@ def _callback_endpoint():
 
 
 class _FakeRequest:
-    """Minimal stand-in for starlette Request — the callback only reads headers."""
-    headers = {"host": "localhost:7000"}
+    """Minimal stand-in for starlette Request — the callback reads the Host header
+    and the request scheme. Behind a TLS terminator uvicorn's proxy-headers
+    middleware rewrites the scheme from `X-Forwarded-Proto`, so the route sees
+    `https` there and `http` on a plain origin."""
+
+    def __init__(self, scheme="http", host="localhost:7000"):
+        self.headers = {"host": host}
+        self.url = SimpleNamespace(scheme=scheme)
 
 
 def _location(resp):
@@ -415,6 +422,119 @@ async def test_callback_valid_owner_writes_encrypted_tokens_to_intended_account(
     assert other.oauth_access_token is None, "tokens must only touch the intended account"
 
 
+# ── Redirect URI scheme ───────────────────────────────────────────
+#
+# Google rejects the token exchange unless the callback's `redirect_uri` is
+# byte-identical to the one the authorize step sent, so both routes have to
+# agree — including on the scheme. Deriving it from the request keeps HTTPS
+# deployments working without pinning GOOGLE_OAUTH_REDIRECT_URI by hand;
+# hardcoding `http://` produced an unusable redirect behind any TLS front.
+
+def _authorize_endpoint():
+    """Return the live google_oauth_authorize endpoint from the email router."""
+    from routes.email_routes import setup_email_routes
+    router = setup_email_routes()
+    for route in router.routes:
+        if route.path == "/api/email/oauth/google/authorize" and "GET" in getattr(route, "methods", set()):
+            return route.endpoint
+    raise AssertionError("google_oauth_authorize route not found")
+
+
+def _posted_redirect_uri(mock_post):
+    """Pull `redirect_uri` out of the mocked Google token-exchange POST."""
+    return mock_post.call_args.kwargs["data"]["redirect_uri"]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("scheme", ("http", "https"))
+async def test_callback_redirect_uri_follows_the_request_scheme(scheme, monkeypatch):
+    """The token exchange must echo the scheme the request actually arrived on —
+    `https` behind a TLS terminator, `http` on a plain origin."""
+    from routes.email_helpers import make_oauth_state
+
+    monkeypatch.delenv("GOOGLE_OAUTH_REDIRECT_URI", raising=False)
+
+    db, Factory = _make_db()
+    _make_account(db, account_id="acct-s", owner="alice", imap_user="alice@example.com")
+    db.close()
+
+    token_resp = mock.MagicMock()
+    token_resp.raise_for_status = mock.MagicMock()
+    token_resp.json.return_value = {"access_token": "ya29.t", "refresh_token": "1//r", "expires_in": 3600}
+    userinfo_resp = mock.MagicMock()
+    userinfo_resp.is_success = True
+    userinfo_resp.json.return_value = {"email": "alice@example.com", "name": "Alice"}
+
+    state = make_oauth_state("acct-s", "alice")
+
+    with mock.patch("httpx.post", return_value=token_resp) as mock_post, \
+         mock.patch("httpx.get", return_value=userinfo_resp), \
+         mock.patch("core.database.SessionLocal", Factory):
+        callback = _callback_endpoint()
+        await callback(
+            code="4/code", state=state, error=None,
+            request=_FakeRequest(scheme=scheme, host="odysseus.example.ts.net:7443"),
+        )
+
+    assert _posted_redirect_uri(mock_post) == (
+        f"{scheme}://odysseus.example.ts.net:7443/api/email/oauth/google/callback"
+    )
+
+
+@pytest.mark.asyncio
+async def test_callback_redirect_uri_env_override_still_wins(monkeypatch):
+    """An explicit GOOGLE_OAUTH_REDIRECT_URI is used verbatim — deriving the
+    scheme must not override a value the operator pinned by hand."""
+    from routes.email_helpers import make_oauth_state
+
+    pinned = "https://mail.example.com/api/email/oauth/google/callback"
+    monkeypatch.setenv("GOOGLE_OAUTH_REDIRECT_URI", pinned)
+
+    db, Factory = _make_db()
+    _make_account(db, account_id="acct-p", owner="alice", imap_user="alice@example.com")
+    db.close()
+
+    token_resp = mock.MagicMock()
+    token_resp.raise_for_status = mock.MagicMock()
+    token_resp.json.return_value = {"access_token": "ya29.t", "refresh_token": "1//r", "expires_in": 3600}
+    userinfo_resp = mock.MagicMock()
+    userinfo_resp.is_success = True
+    userinfo_resp.json.return_value = {"email": "alice@example.com", "name": "Alice"}
+
+    state = make_oauth_state("acct-p", "alice")
+
+    with mock.patch("httpx.post", return_value=token_resp) as mock_post, \
+         mock.patch("httpx.get", return_value=userinfo_resp), \
+         mock.patch("core.database.SessionLocal", Factory):
+        callback = _callback_endpoint()
+        await callback(code="4/code", state=state, error=None, request=_FakeRequest(scheme="http"))
+
+    assert _posted_redirect_uri(mock_post) == pinned
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("scheme", ("http", "https"))
+async def test_authorize_redirect_uri_follows_the_request_scheme(scheme, monkeypatch):
+    """The authorize step builds the same redirect_uri the callback will send.
+    `owner=""` is the unconfigured / single-user case, so no DB is touched."""
+    import urllib.parse
+
+    monkeypatch.delenv("GOOGLE_OAUTH_REDIRECT_URI", raising=False)
+    monkeypatch.setenv("GOOGLE_OAUTH_CLIENT_ID", "client-id.apps.googleusercontent.com")
+
+    authorize = _authorize_endpoint()
+    resp = await authorize(
+        account_id="acct-a",
+        request=_FakeRequest(scheme=scheme, host="odysseus.example.ts.net:7443"),
+        owner="",
+    )
+
+    query = urllib.parse.parse_qs(urllib.parse.urlparse(resp.headers["location"]).query)
+    assert query["redirect_uri"] == [
+        f"{scheme}://odysseus.example.ts.net:7443/api/email/oauth/google/callback"
+    ]
+
+
 @pytest.mark.asyncio
 async def test_callback_rejects_token_for_a_different_mailbox_identity():
     """Reconnecting with another Google identity must not replace the token

tests/test_gallery_model_input_device.py

@@ -0,0 +1,34 @@
+import routes.gallery_routes as gallery_routes
+
+
+class _TorchSentinel:
+    float32 = object()
+    float64 = object()
+
+
+class _FakeTensor:
+    def __init__(self, dtype):
+        self.dtype = dtype
+        self.to_args = None
+
+    def to(self, *args, **kwargs):
+        self.to_args = (args, kwargs)
+        return self
+
+
+def test_model_inputs_to_device_casts_mps_float64_to_float32():
+    float_tensor = _FakeTensor(_TorchSentinel.float64)
+    int_tensor = _FakeTensor("int64")
+    plain_value = object()
+
+    result = gallery_routes._model_inputs_to_device(
+        {"points": float_tensor, "labels": int_tensor, "plain": plain_value},
+        "mps",
+        _TorchSentinel,
+    )
+
+    assert result["points"] is float_tensor
+    assert float_tensor.to_args == ((), {"device": "mps", "dtype": _TorchSentinel.float32})
+    assert int_tensor.to_args == (("mps",), {})
+    assert result["plain"] is plain_value
+

tests/test_harmony_tool_aliasing.py

@@ -0,0 +1,78 @@
+"""Tools whose names collide with harmony built-ins must be aliased for gpt-oss.
+
+gpt-oss (harmony format) ships BUILT-IN tools named `python` and `browser`,
+invoked with the raw body as the argument (`to=python` + bare source), while
+custom functions use `to=functions.NAME` + JSON. Exposing our own tool under a
+built-in's name makes the model answer with the built-in convention: it emits
+raw code, the server tries to parse it as JSON, and the request dies with
+"error parsing tool call: raw='import sys, ...'". Streaming is worse — Ollama
+truncates the stream instead of reporting it, so the turn looks like an empty
+response and the agent loop reads it as a stall.
+
+Measured on gpt-oss:20b via Ollama /v1 with a fixed agentic prompt:
+python+bash as-is 2/6, python renamed 5/6, both renamed 6/6.
+
+The aliasing is transport-only and gpt-oss-only: every other model's tool
+schemas must pass through untouched, and real tool names must come back out.
+"""
+from src.llm_core import (
+    _alias_harmony_tools,
+    _unalias_harmony_tool_name,
+    _is_harmony_model,
+)
+
+
+def _tools(*names):
+    return [
+        {"type": "function", "function": {"name": n, "parameters": {}}}
+        for n in names
+    ]
+
+
+def _names(tools):
+    return [t["function"]["name"] for t in tools]
+
+
+def test_gpt_oss_colliding_names_are_aliased():
+    out = _alias_harmony_tools(_tools("python", "bash", "web_search"), "gpt-oss:20b")
+    assert _names(out) == ["run_python_code", "run_shell_command", "web_search"]
+
+
+def test_non_harmony_models_are_untouched():
+    tools = _tools("python", "bash", "web_search")
+    for model in ("qwen3-coder:30b", "gemma4:12b", "claude-opus-5", "gpt-4o", "llama-3.3"):
+        out = _alias_harmony_tools(tools, model)
+        assert _names(out) == ["python", "bash", "web_search"], model
+        assert out is tools, f"{model} should get the same list object, not a copy"
+
+
+def test_aliasing_does_not_mutate_the_caller_list():
+    tools = _tools("python")
+    _alias_harmony_tools(tools, "gpt-oss:20b")
+    assert _names(tools) == ["python"], "caller's schema list must not be mutated"
+
+
+def test_response_names_map_back_for_gpt_oss():
+    assert _unalias_harmony_tool_name("run_python_code", "gpt-oss:20b") == "python"
+    assert _unalias_harmony_tool_name("run_shell_command", "gpt-oss:20b") == "bash"
+    # Unrelated names pass through untouched.
+    assert _unalias_harmony_tool_name("web_search", "gpt-oss:20b") == "web_search"
+
+
+def test_response_names_untouched_for_other_models():
+    # A non-harmony model that genuinely has a tool called run_python_code
+    # must not have it rewritten to `python`.
+    assert _unalias_harmony_tool_name("run_python_code", "qwen3-coder:30b") == "run_python_code"
+
+
+def test_harmony_detection():
+    assert _is_harmony_model("gpt-oss:20b") is True
+    assert _is_harmony_model("GPT-OSS:120B") is True
+    assert _is_harmony_model("qwen3-coder:30b") is False
+    assert _is_harmony_model("") is False
+    assert _is_harmony_model(None) is False
+
+
+def test_empty_and_none_tools_are_safe():
+    assert _alias_harmony_tools(None, "gpt-oss:20b") is None
+    assert _alias_harmony_tools([], "gpt-oss:20b") == []

tests/test_llm_core_async_mistral_content.py

@@ -0,0 +1,71 @@
+"""Integration regression test for #5435.
+
+llm_call_async must normalise Mistral structured content to a plain string,
+matching llm_call (sync) and stream_llm. Before the fix, the async
+non-streaming parser returned the raw list when Mistral reasoning was enabled,
+violating its -> str contract, leaking a non-string into callers such as
+auto-title generation and memory extraction, and poisoning _response_cache
+with a non-string value.
+"""
+import asyncio
+
+import src.llm_core as llm_core
+
+
+class _FakeResponse:
+    is_success = True
+    status_code = 200
+    text = ""
+
+    def __init__(self, payload):
+        self._payload = payload
+
+    def json(self):
+        return self._payload
+
+
+def _payload(content):
+    return {"choices": [{"message": {"role": "assistant", "content": content}}]}
+
+
+def _call(monkeypatch, content):
+    async def fake_post(client, url, headers, **kwargs):
+        return _FakeResponse(_payload(content))
+
+    monkeypatch.setattr(llm_core, "httpx_post_kimi_aware_async", fake_post)
+    llm_core._response_cache.clear()
+    return asyncio.run(llm_core.llm_call_async(
+        "http://mistral.test/v1/chat/completions",
+        "mistral-medium",
+        [{"role": "user", "content": "q"}],
+    ))
+
+
+def test_llm_call_async_normalizes_mistral_structured_content(monkeypatch):
+    out = _call(monkeypatch, [
+        {"type": "thinking",
+         "thinking": [{"type": "text", "text": "Let me work through this..."}],
+         "closed": True},
+        {"type": "text", "text": "The answer is 42."},
+    ])
+    assert isinstance(out, str), f"expected str, got {type(out).__name__}"
+    assert "The answer is 42." in out
+    assert "Let me work through this..." in out
+    # The cache must hold the normalised string, not the raw list,
+    # otherwise repeat calls serve the poisoned value.
+    assert all(isinstance(v, str) for v in llm_core._response_cache.values())
+
+
+def test_llm_call_async_thinking_only_still_returns_str(monkeypatch):
+    out = _call(monkeypatch, [
+        {"type": "thinking",
+         "thinking": [{"type": "text", "text": "still thinking"}],
+         "closed": True},
+    ])
+    assert isinstance(out, str)
+    assert "still thinking" in out
+
+
+def test_llm_call_async_plain_string_passthrough(monkeypatch):
+    out = _call(monkeypatch, "plain answer")
+    assert out == "plain answer"

tests/test_skill_frontmatter_escape_roundtrip.py

@@ -0,0 +1,214 @@
+"""Regression for issue #5210 — SKILL.md frontmatter scalars must round-trip.
+
+``_emit_scalar`` quotes a scalar with ``json.dumps`` whenever it contains
+punctuation that would change how the line reads back. ``_parse_scalar`` used
+to undo that with a bare ``raw[1:-1]``: it stripped the quotes but never
+decoded the escapes. So ``"Pr\\u00fcfung"`` was read back as the literal text
+``Pr\\u00fcfung``, and the next save escaped *that* backslash again.
+
+The damage compounds — each save doubles the backslash run — so a German or
+Japanese skill description degrades into backslash noise after a handful of
+edits, and the same happens to a plain-ASCII description that merely contains
+a quote character. The escapes are also shown verbatim in the skills list and
+the ``/skills`` catalog.
+
+The fix is symmetric: emit with ``ensure_ascii=False`` (SKILL.md is UTF-8 at
+both ends) and parse double-quoted scalars with ``json.loads``.
+"""
+
+import json
+
+import pytest
+
+from services.memory.skill_format import (
+    Skill,
+    _emit_scalar,
+    _parse_scalar,
+    parse_frontmatter,
+)
+from services.memory.skills import SkillsManager
+
+# Umlauts plus a comma — the comma is what forces the quoted form, which is the
+# only path that was corrupted. Reported verbatim in issue #5210.
+GERMAN = "Einstiegs- und Pr\u00fcfungslinie f\u00fcr AGB, Datenschutz"
+JAPANESE = "\u30b9\u30ad\u30eb: \u30c6\u30b9\u30c8\u7528\u306e\u8aac\u660e"
+QUOTED_ASCII = 'Use the "grep" tool, then summarise'
+
+
+def _cycle(skill: Skill, times: int = 1) -> Skill:
+    """Save to markdown and read it straight back, `times` times over."""
+    for _ in range(times):
+        skill = Skill.from_markdown(skill.to_markdown())
+    return skill
+
+
+# ---------------------------------------------------------------------------
+# The reported corruption
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    "description",
+    [GERMAN, JAPANESE, QUOTED_ASCII],
+    ids=["german", "japanese", "quoted-ascii"],
+)
+def test_description_survives_repeated_saves(description):
+    """Five load/save cycles must leave the text byte-identical.
+
+    One cycle is enough to corrupt it; five is where the doubling became
+    obvious in the field.
+    """
+    result = _cycle(Skill(name="demo", description=description), times=5)
+    assert result.description == description
+
+
+def test_corruption_does_not_compound_across_saves():
+    """Pin the *growth*, not just the mismatch.
+
+    The original defect was not a one-off mangling — the escaped form was
+    re-escaped on every save, so the value grew without bound. A regression
+    that reintroduced single-level mangling would still be caught by the test
+    above; this one catches the runaway specifically.
+    """
+    skill = Skill(name="demo", description=GERMAN)
+    lengths = []
+    for _ in range(5):
+        skill = _cycle(skill)
+        lengths.append(len(skill.description))
+    assert len(set(lengths)) == 1, f"description length drifted across saves: {lengths}"
+
+
+# ---------------------------------------------------------------------------
+# What actually lands on disk
+# ---------------------------------------------------------------------------
+
+
+def test_non_ascii_is_written_as_utf8_not_ascii_escapes():
+    """SKILL.md is opened as UTF-8 at both ends, so \\uXXXX buys nothing and
+    only makes the file unreadable to a human editing it."""
+    markdown = Skill(name="demo", description=GERMAN).to_markdown()
+    line = next(l for l in markdown.splitlines() if l.startswith("description:"))
+    assert "Pr\u00fcfungslinie" in line
+    assert "\\u00fc" not in line
+
+
+def test_quoted_scalar_is_valid_json():
+    """The emitted form is what the parser now feeds to json.loads, so the two
+    halves cannot drift apart without this failing."""
+    emitted = _emit_scalar(QUOTED_ASCII)
+    assert json.loads(emitted) == QUOTED_ASCII
+
+
+# ---------------------------------------------------------------------------
+# Existing files
+# ---------------------------------------------------------------------------
+
+
+def test_legacy_ascii_escaped_file_is_read_correctly():
+    """Files already written by the old emitter hold real JSON escapes, so the
+    new parser recovers the intended text instead of the escape source."""
+    markdown = '---\nname: demo\ndescription: "Pr\\u00fcfung, x"\n---\n\n'
+    assert Skill.from_markdown(markdown).description == "Pr\u00fcfung, x"
+
+
+def test_already_corrupted_file_heals_one_level_per_load():
+    """A file that took one round of damage carries a doubled backslash. That
+    is still valid JSON, so reading it yields the single-backslash form and the
+    value stops degrading."""
+    markdown = '---\nname: demo\ndescription: "Pr\\\\u00fcfung, x"\n---\n\n'
+    once = Skill.from_markdown(markdown)
+    assert once.description == "Pr\\u00fcfung, x"
+    # And it is now stable rather than growing on every subsequent save.
+    assert _cycle(once, times=3).description == "Pr\\u00fcfung, x"
+
+
+def test_non_json_escape_falls_back_to_literal_reading():
+    """A hand-written frontmatter value can hold escapes JSON rejects (a bare
+    Windows path is the common one). Those must keep their previous literal
+    reading rather than raising."""
+    assert _parse_scalar('"C:\\Users\\demo"') == "C:\\Users\\demo"
+
+
+# ---------------------------------------------------------------------------
+# Unchanged behaviour
+# ---------------------------------------------------------------------------
+
+
+def test_plain_scalars_are_still_emitted_bare():
+    """Only values needing quotes get them — the common case must not suddenly
+    start quoting, which would churn every SKILL.md on disk."""
+    assert _emit_scalar("open-pr-from-branch") == "open-pr-from-branch"
+    assert _emit_scalar("1.0.0") == "1.0.0"
+    assert _emit_scalar(True) == "true"
+    assert _emit_scalar(None) == "null"
+    assert _emit_scalar(0.8) == "0.8"
+
+
+def test_single_quoted_scalar_keeps_literal_reading():
+    """Only double-quoted scalars are JSON. Single-quoted ones are read the way
+    they always were."""
+    assert _parse_scalar("'plain, text'") == "plain, text"
+
+
+def test_lists_round_trip_with_non_ascii_entries():
+    skill = Skill(name="demo", tags=["b\u00fcro", "recht, steuern"])
+    assert _cycle(skill, times=3).tags == ["b\u00fcro", "recht, steuern"]
+
+
+# ---------------------------------------------------------------------------
+# Line-break characters
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    "sep",
+    ["\u2028", "\u2029", "\x85", "\r", "\v", "\f", "\x1c"],
+    ids=["ls", "ps", "nel", "cr", "vt", "ff", "fs"],
+)
+def test_line_break_characters_do_not_split_the_frontmatter(sep):
+    """parse_frontmatter() reads one scalar per line via str.splitlines(),
+    which breaks on far more than \\n. Any of these landing unescaped in the
+    file would silently truncate the value and shift the remainder into a
+    bogus key.
+
+    json.dumps covers the C0 ones, but with ensure_ascii=False it passes NEL,
+    LINE SEPARATOR and PARAGRAPH SEPARATOR through as themselves — so those
+    three are re-escaped explicitly.
+    """
+    description = f"before{sep}after, x"
+    markdown = Skill(name="demo", description=description).to_markdown()
+
+    frontmatter_text = markdown.split("---")[1]
+    assert len(frontmatter_text.strip().splitlines()) == len(
+        [l for l in frontmatter_text.strip().split("\n") if l.strip()]
+    ), "a scalar leaked a line break into the frontmatter"
+
+    fm, _body = parse_frontmatter(markdown)
+    assert fm["description"] == description
+
+
+# ---------------------------------------------------------------------------
+# End to end, through real files
+# ---------------------------------------------------------------------------
+
+
+def test_description_survives_real_save_load_cycles_on_disk(tmp_path):
+    """The unit tests above go straight through to_markdown/from_markdown.
+    This drives the same path the app does — SkillsManager writing UTF-8 files
+    with atomic_write_text and reading them back — because the encoding used at
+    either end is part of the fix.
+    """
+    manager = SkillsManager(str(tmp_path))
+    manager.add_skill(name="agb-pruefung", description=GERMAN, category="general")
+
+    for _ in range(4):
+        assert manager.update_skill("agb-pruefung", {"status": "published"})
+
+    stored = [s for s in manager.load_all() if s["name"] == "agb-pruefung"]
+    assert len(stored) == 1
+    assert stored[0]["description"] == GERMAN
+
+    on_disk = (tmp_path / "skills" / "general" / "agb-pruefung" / "SKILL.md").read_text(
+        encoding="utf-8"
+    )
+    assert GERMAN in on_disk

tests/test_spinner_stops_when_never_attached_js.py

@@ -0,0 +1,319 @@
+"""Pin the self-termination contract of the canvas spinners in static/js/spinner.js.
+
+Background: the whirlpool spinner drives itself with requestAnimationFrame and
+decides whether to keep going by looking at `element.isConnected`. It used to
+re-arm forever whenever the element had *never* been connected, on the theory
+that start() runs before the caller appends the element. Callers that start a
+spinner and then take an early return - an aborted request, a panel that
+resolved from cache before the loading row was inserted - therefore left a rAF
+loop redrawing an 84-segment spiral into a detached canvas until the tab closed.
+Measured on an idle app: ~110 whirlpool frames per second with zero canvases in
+the document.
+
+These tests lock in all four exits (never attached, attached-then-removed,
+stop(), tab hidden) and, just as importantly, the one case that must NOT stop:
+a spinner that is actually on screen.
+
+Driven through `node --input-type=module` so the real module runs, same idiom as
+test_esc_menu_stack_js.py. The module source is inlined rather than imported by
+path because the repo has no `"type": "module"` in package.json; spinner.js has
+no imports of its own, so inlining is exact. A fake clock and a manual frame
+pump replace performance.now()/requestAnimationFrame, so nothing here depends on
+wall-clock time or real frame timing.
+"""
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+_REPO = Path(__file__).resolve().parent.parent
+_MODULE = _REPO / "static" / "js" / "spinner.js"
+_HAS_NODE = shutil.which("node") is not None
+_SRC = _MODULE.read_text(encoding="utf-8") if _MODULE.exists() else ""
+
+# Browser stand-ins, installed before the module body runs. `clock` is advanced
+# only by pump(), so every timing decision in the module is deterministic.
+_STUBS = r"""
+let clock = 0;
+Object.defineProperty(globalThis, 'performance', {
+  value: { now: () => clock }, configurable: true, writable: true,
+});
+
+const pending = new Map();
+let nextFrameId = 1;
+let framesRun = 0;
+globalThis.requestAnimationFrame = (cb) => {
+  const id = nextFrameId++;
+  pending.set(id, cb);
+  return id;
+};
+globalThis.cancelAnimationFrame = (id) => { pending.delete(id); };
+
+/** Advance the clock `steps` frames of `msPerFrame` and run whatever is queued. */
+function pump(steps, msPerFrame = 16) {
+  for (let i = 0; i < steps; i++) {
+    clock += msPerFrame;
+    const due = [...pending.values()];
+    pending.clear();
+    for (const cb of due) { framesRun++; cb(); }
+  }
+}
+function framesPending() { return pending.size; }
+function framesSince(mark) { return framesRun - mark; }
+function frameMark() { return framesRun; }
+
+function makeCtx() {
+  const noop = () => {};
+  return {
+    clearRect: noop, beginPath: noop, arc: noop, moveTo: noop, lineTo: noop,
+    stroke: noop, fill: noop, save: noop, restore: noop,
+    strokeStyle: '', fillStyle: '', lineWidth: 0, globalAlpha: 1,
+    lineCap: '', lineJoin: '',
+  };
+}
+
+function makeElement(tag) {
+  const el = {
+    tagName: tag, className: '', textContent: '', innerHTML: '',
+    width: 0, height: 0, isConnected: false, parentNode: null,
+    style: { cssText: '' },
+    children: [],
+    classList: { add: () => {}, remove: () => {}, contains: () => false },
+    getContext: () => makeCtx(),
+    appendChild(child) {
+      child.parentNode = this;
+      this.children.push(child);
+      return child;
+    },
+    removeChild(child) {
+      this.children = this.children.filter((c) => c !== child);
+      child.parentNode = null;
+      return child;
+    },
+  };
+  return el;
+}
+
+const docListeners = [];
+globalThis.document = {
+  hidden: false,
+  documentElement: makeElement('html'),
+  createElement: makeElement,
+  createTextNode: (t) => ({ textContent: t }),
+  addEventListener: (type, fn) => { docListeners.push([type, fn]); },
+  removeEventListener: (type, fn) => {
+    const i = docListeners.findIndex(([t, f]) => t === type && f === fn);
+    if (i >= 0) docListeners.splice(i, 1);
+  },
+};
+globalThis.getComputedStyle = () => ({ getPropertyValue: () => '' });
+
+function visibilityListeners() {
+  return docListeners.filter(([t]) => t === 'visibilitychange').length;
+}
+function fireVisibility(hidden) {
+  document.hidden = hidden;
+  for (const [t, fn] of [...docListeners]) if (t === 'visibilitychange') fn();
+}
+
+/** A started whirlpool spinner whose element is not in the document. */
+function startedWhirlpool() {
+  const sp = new Spinner('', 'clean', 'whirlpool');
+  sp.createElement();
+  sp.start();
+  return sp;
+}
+"""
+
+
+def _run(body: str) -> dict:
+    """Run `body` with the real spinner module and the browser stubs in scope."""
+    js = _STUBS + "\n" + _SRC + "\n" + body
+    proc = subprocess.run(
+        ["node", "--input-type=module"],
+        input=js, capture_output=True, text=True, encoding="utf-8",
+        cwd=str(_REPO), timeout=30,
+    )
+    assert proc.returncode == 0, proc.stderr
+    return json.loads(proc.stdout.strip())
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_never_attached_whirlpool_stops_itself():
+    # The leak: element created, spinner started, element never inserted. Past
+    # the grace window it must give up rather than re-arm forever.
+    body = """
+    const sp = startedWhirlpool();
+    pump(30);                       // 480 ms - inside the grace window
+    const early = { running: sp.isRunning, pending: framesPending() };
+    pump(120);                      // ~2.4 s total - past the grace window
+    const mark = frameMark();
+    pump(60);                       // nothing should be left to run
+    console.log(JSON.stringify({
+      early,
+      running: sp.isRunning,
+      rafId: sp.rafId,
+      pending: framesPending(),
+      framesAfterStop: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert out["early"] == {"running": True, "pending": 1}, "gave up during the grace window"
+    assert out["running"] is False
+    assert out["rafId"] is None
+    assert out["pending"] == 0
+    assert out["framesAfterStop"] == 0, "loop kept drawing after it gave up"
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_attached_whirlpool_keeps_running_past_the_grace_window():
+    # The converse guard: the fix must not kill spinners that are on screen.
+    body = """
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    pump(400);                      // ~6.4 s, far past the grace window
+    const mark = frameMark();
+    pump(10);
+    console.log(JSON.stringify({
+      running: sp.isRunning,
+      pending: framesPending(),
+      framesDrawn: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert out["running"] is True
+    assert out["pending"] == 1
+    assert out["framesDrawn"] == 10, "a visible spinner stopped animating"
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_attached_then_removed_whirlpool_stops():
+    # The pre-existing exit - a loading row replaced by results - still works.
+    body = """
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    pump(200);
+    const whileAttached = sp.isRunning;
+    sp.element.isConnected = false; // results arrived, row swapped out
+    pump(3);
+    const mark = frameMark();
+    pump(20);
+    console.log(JSON.stringify({
+      whileAttached,
+      running: sp.isRunning,
+      pending: framesPending(),
+      framesAfterRemoval: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert out["whileAttached"] is True
+    assert out["running"] is False
+    assert out["pending"] == 0
+    assert out["framesAfterRemoval"] == 0
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_loading_row_helper_stops_when_the_row_is_never_inserted():
+    # createLoadingRow() starts the spinner for the caller and hands back a
+    # detached row, so a caller that early-returns is the real leak shape.
+    body = """
+    const row = createLoadingRow('Loading...', 16);
+    pump(200);
+    const mark = frameMark();
+    pump(40);
+    console.log(JSON.stringify({
+      pending: framesPending(),
+      framesAfterStop: framesSince(mark),
+      rowHasChildren: row.children.length > 0,
+    }));
+    """
+    out = _run(body)
+    assert out["rowHasChildren"] is True, "harness built the wrong row"
+    assert out["pending"] == 0
+    assert out["framesAfterStop"] == 0
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_stop_cancels_the_pending_frame_and_releases_the_listener():
+    # stop() must be authoritative: no queued frame survives it, and it leaves
+    # no visibilitychange listener behind on a dead spinner.
+    body = """
+    const before = visibilityListeners();
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    const armed = visibilityListeners();
+    sp.stop();
+    const mark = frameMark();
+    pump(20);
+    console.log(JSON.stringify({
+      before, armed, after: visibilityListeners(),
+      running: sp.isRunning,
+      rafId: sp.rafId,
+      pending: framesPending(),
+      framesAfterStop: framesSince(mark),
+    }));
+    """
+    out = _run(body)
+    assert (out["before"], out["armed"], out["after"]) == (0, 1, 0)
+    assert out["running"] is False
+    assert out["rafId"] is None
+    assert out["pending"] == 0
+    assert out["framesAfterStop"] == 0
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_hidden_tab_pauses_frames_and_showing_resumes_them():
+    body = """
+    const sp = new Spinner('', 'clean', 'whirlpool');
+    sp.createElement();
+    sp.element.isConnected = true;
+    sp.start();
+    pump(5);
+    fireVisibility(true);
+    const hiddenMark = frameMark();
+    pump(30);
+    const whileHidden = { drawn: framesSince(hiddenMark), pending: framesPending() };
+    fireVisibility(false);
+    const shownMark = frameMark();
+    pump(10);
+    console.log(JSON.stringify({
+      whileHidden,
+      running: sp.isRunning,
+      drawnAfterShow: framesSince(shownMark),
+    }));
+    """
+    out = _run(body)
+    assert out["whileHidden"] == {"drawn": 0, "pending": 0}, "kept drawing in a hidden tab"
+    assert out["running"] is True
+    assert out["drawnAfterShow"] == 10, "did not resume when the tab came back"
+
+
+@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+def test_restarted_spinner_gets_a_fresh_grace_window():
+    # The grace deadline is per-run. A spinner reused after stop() must not
+    # inherit the previous run's timestamp and die on its first frame.
+    body = """
+    const sp = startedWhirlpool();
+    pump(200);                      // times out, never attached
+    const stopped = sp.isRunning;
+    sp.element.isConnected = true;  // now inserted for real
+    sp.start();
+    pump(30);
+    console.log(JSON.stringify({
+      stopped,
+      running: sp.isRunning,
+      pending: framesPending(),
+    }));
+    """
+    out = _run(body)
+    assert out["stopped"] is False
+    assert out["running"] is True
+    assert out["pending"] == 1

tests/test_tool_parsing_bare_end_marker.py

@@ -34,6 +34,7 @@ KEPT = [
     ("append end", "append end"),
     ("END", "END"),
     ("\nEnd\n", "End"),
+    ("x assistant y", "x assistant y"),          # mid-sentence must survive (#5971)
 ]
 
 # Real markers — at least one pipe, plus the role word — with the exact output
@@ -44,7 +45,9 @@ STRIPPED = [
     ("a /|end| b", "a  b"),
     ("a |end b", "a  b"),
     ("a end| b", "a  b"),
-    ("x assistant y", "x  y"),
+    ("Before\nassistant\nAfter", "Before \nAfter"),   # bare-marker on its own line still stripped
+    ("Before\n  assistant\t \nAfter", "Before \nAfter"),     # whitespace-padded marker still stripped
+    ("Before\n\tassistan  \nAfter", "Before \nAfter"),       # truncated marker variant still stripped
 ]
 
 

tests/test_tool_parsing_hermes_json.py

@@ -0,0 +1,94 @@
+"""Qwen/Hermes text-mode tool calls: bare JSON inside <tool_call> wrappers.
+
+Issue #5187: <tool_call>{"name": "bash", "arguments": {...}}</tool_call>
+parsed to zero blocks because wrapper bodies were only fed to the XML
+iterators. The JSON body form now parses through the same canonical
+function_call_to_tool_block converter as the XML paths, and JSON-looking
+bodies fail closed instead of falling through to XML scanning (tracker #5333):
+XML-like text inside JSON argument values must stay data, and a non-object
+"arguments" value is rejected rather than coerced.
+"""
+import src.agent_tools  # noqa: F401  (break agent_tools<->tool_parsing import cycle)
+from src.tool_parsing import parse_tool_blocks, strip_tool_blocks
+
+# Verbatim payload from issue #5187.
+ISSUE_PAYLOAD = '<tool_call>\n{"name": "bash", "arguments": {"command": "mkdir -p agent-test"}}\n</tool_call>'
+
+
+def test_issue_5187_payload_parses():
+    blocks = parse_tool_blocks(ISSUE_PAYLOAD)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "bash"
+    assert blocks[0].content == "mkdir -p agent-test"
+
+
+def test_multiple_sequential_wrappers():
+    text = (
+        '<tool_call>\n{"name": "bash", "arguments": {"command": "ls"}}\n</tool_call>\n'
+        'Now the second step:\n'
+        '<tool_call>\n{"name": "bash", "arguments": {"command": "pwd"}}\n</tool_call>'
+    )
+    blocks = parse_tool_blocks(text)
+    assert [(b.tool_type, b.content) for b in blocks] == [("bash", "ls"), ("bash", "pwd")]
+
+
+def test_unclosed_wrapper_still_parses():
+    text = '<tool_call>\n{"name": "bash", "arguments": {"command": "ls -la"}}'
+    blocks = parse_tool_blocks(text)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "bash"
+    assert blocks[0].content == "ls -la"
+
+
+def test_xml_inside_json_arguments_stays_data():
+    # P1: a valid JSON body whose argument values contain XML-like tool markup
+    # must parse as the JSON-named tool; the embedded markup is content.
+    text = (
+        '<tool_call>{"name": "write_file", "arguments": '
+        '{"path": "notes.txt", "content": "<bash>echo unsafe</bash>"}}</tool_call>'
+    )
+    blocks = parse_tool_blocks(text)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "write_file"
+    assert "<bash>echo unsafe</bash>" in blocks[0].content
+    assert all(b.tool_type != "bash" for b in blocks)
+
+
+def test_malformed_json_body_never_falls_through_to_xml():
+    # P1 fail-closed: a JSON-looking body that doesn't decode must not be
+    # rescanned as XML, even when it contains well-formed tool markup.
+    text = (
+        '<tool_call>{"name": "write_file", "arguments": {broken json '
+        '<invoke name="bash"><parameter name="command">echo unsafe</parameter></invoke>'
+        '</tool_call>'
+    )
+    assert parse_tool_blocks(text) == []
+
+
+def test_non_dict_arguments_rejected():
+    # P2: "arguments" must be an object; scalars/arrays are rejected, not coerced.
+    for args in ('["ls"]', '"ls"', '1', 'null'):
+        text = '<tool_call>{"name": "bash", "arguments": %s}</tool_call>' % args
+        assert parse_tool_blocks(text) == [], f"arguments={args} should be rejected"
+
+
+def test_strip_tool_blocks_removes_json_wrapper_spans():
+    text = "Before.\n" + ISSUE_PAYLOAD + "\nAfter."
+    cleaned = strip_tool_blocks(text)
+    assert "tool_call" not in cleaned
+    assert "mkdir -p agent-test" not in cleaned
+    assert "Before." in cleaned
+    assert "After." in cleaned
+
+
+def test_xml_body_wrapper_regression():
+    # The pre-existing XML wrapper form must keep parsing exactly as before.
+    text = (
+        '<tool_call><invoke name="bash">'
+        '<parameter name="command">echo hi</parameter>'
+        '</invoke></tool_call>'
+    )
+    blocks = parse_tool_blocks(text)
+    assert len(blocks) == 1
+    assert blocks[0].tool_type == "bash"
+    assert blocks[0].content == "echo hi"

tests/test_ui_visibility_js.py

@@ -0,0 +1,139 @@
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+
+ROOT = Path(__file__).resolve().parents[1]
+pytestmark = pytest.mark.skipif(not shutil.which("node"), reason="node binary not on PATH")
+
+
+def _node_eval(source):
+    result = subprocess.run(
+        ["node", "--input-type=module", "-e", source],
+        cwd=ROOT,
+        check=True,
+        capture_output=True,
+        text=True,
+    )
+    return json.loads(result.stdout)
+
+
+def _resolve(state):
+    """Run resolveVisibility(state) in node; return {selector: visible}."""
+    return _node_eval(
+        f"""
+        const {{ resolveVisibility }} = await import('./static/js/ui_visibility.js');
+        console.log(JSON.stringify(resolveVisibility({json.dumps(state)})));
+        """
+    )
+
+
+def _map():
+    return _node_eval(
+        """
+        const { UI_VIS_MAP } = await import('./static/js/ui_visibility.js');
+        console.log(JSON.stringify(UI_VIS_MAP));
+        """
+    )
+
+
+# Selectors (kept in one place so the tests read as plain assertions).
+EMAIL = "#email-section, #rail-email"
+TOOLS = "#tools-section"
+CAL = "#tool-calendar-btn, #rail-calendar"
+COMPARE = "#tool-compare-btn, #rail-compare"
+LIB = "#tool-library-btn, #rail-archive"
+RESEARCH = "#tool-research-btn, #rail-research"
+NEWCHAT = "#rail-new-session"
+RAG = "#overflow-rag-btn"
+
+# Full-sidebar tabs that have an icon-rail counterpart must pair it into their
+# UI_VIS_MAP selector; otherwise minimizing the sidebar re-shows a tab the user
+# turned off in the full view (#tool-library-btn's rail counterpart is #rail-archive).
+EXPECTED_RAIL_PAIRS = {
+    "email-section": "#rail-email",
+    "tool-calendar": "#rail-calendar",
+    "tool-compare": "#rail-compare",
+    "tool-cookbook": "#rail-cookbook",
+    "tool-research": "#rail-research",
+    "tool-gallery": "#rail-gallery",
+    "tool-library": "#rail-archive",
+    "tool-memory": "#rail-memory",
+    "tool-notes": "#rail-notes",
+    "tool-tasks": "#rail-tasks",
+    "tool-theme": "#rail-theme",
+}
+
+
+def test_every_customizable_tab_pairs_its_rail_button():
+    ui_vis_map = _map()
+    missing = {
+        key: rail
+        for key, rail in EXPECTED_RAIL_PAIRS.items()
+        if rail not in ui_vis_map.get(key, "")
+    }
+    assert not missing, (
+        "these tabs are missing their icon-rail counterpart in UI_VIS_MAP "
+        f"(minimizing the sidebar would re-show them): {missing}"
+    )
+
+
+def test_defaults_everything_visible_except_default_off():
+    m = _resolve({})
+    assert m[EMAIL] is True
+    assert m[TOOLS] is True
+    assert m[CAL] is True
+    assert m[NEWCHAT] is True
+    assert m[RAG] is False  # rag-toggle-btn is default-off
+
+
+def test_email_off_hides_email_and_its_rail_only():
+    m = _resolve({"email-section": False})
+    assert m[EMAIL] is False
+    assert m[CAL] is True
+    assert m[TOOLS] is True
+
+
+def test_tool_off_hides_its_rail_launcher():
+    m = _resolve({"tool-calendar": False})
+    assert m[CAL] is False
+    assert m[COMPARE] is True
+
+
+def test_library_off_hides_archive_rail():
+    # tool-library's rail counterpart is #rail-archive (mirrors _railToolMap).
+    m = _resolve({"tool-library": False})
+    assert m[LIB] is False
+
+
+def test_tools_off_hides_every_tool_rail_but_not_email():
+    m = _resolve({"tools-section": False})
+    assert m[TOOLS] is False
+    for sel in (CAL, COMPARE, LIB, RESEARCH):
+        assert m[sel] is False, sel
+    assert m[EMAIL] is True  # email is independent of the Tools section
+
+
+def test_tools_off_overrides_per_tool_on():
+    # A tool individually "on" must still hide when its parent Tools is off.
+    m = _resolve({"tools-section": False, "tool-calendar": True})
+    assert m[CAL] is False
+
+
+def test_tools_on_with_tool_off_hides_only_that_tool():
+    m = _resolve({"tools-section": True, "tool-research": False})
+    assert m[RESEARCH] is False
+    assert m[CAL] is True
+
+
+def test_rail_new_chat_off_hides_new_session():
+    m = _resolve({"rail-new-chat": False})
+    assert m[NEWCHAT] is False
+
+
+def test_explicit_false_takes_precedence_over_default_on():
+    m = _resolve({"rag-toggle-btn": True})
+    assert m[RAG] is True