Review — end-to-end ❌ (request changes — /sw.js change is a subpath-mount regression; other 3 fixes look good)

Thanks @bergeouss for bundling 4 P0 fixes. Three of them are reasonable and should land — the fourth (/sw.js absolute path) is unfortunately a regression for installs behind a reverse proxy at a subpath. Details on each below.

Fix 1 — /sw.js path change ❌ blocking regression

The change (static/index.html:50):

- navigator.serviceWorker.register('sw.js?v=__WEBUI_VERSION__').catch(...)
+ navigator.serviceWorker.register('/sw.js?v=__WEBUI_VERSION__').catch(...)

Why this breaks subpath-mount installs. static/index.html:16-17 explicitly documents the convention:

<!-- base href enables subpath mount support; all static paths must stay relative (no leading slash) -->
<script>(function(){var path=location.pathname,marker='/session/',i=path.indexOf(marker),p;i>=0?p=(path.slice(0,i+1)||'/'):p=(path.endsWith('/')?path:(path.replace(/\/[^\/]*$/,'/')||'/'));document.write('<base href="'+location.origin+p+'">');})()</script>

The dynamic <base href> is in place specifically so installs behind nginx/Apache at e.g. /hermes/ resolve every static asset URL correctly. Verified the logic with a Node harness across the URL shapes hermes-webui actually serves:

/                   -> base /,        sw.js -> /sw.js                 ✓
/index.html         -> base /,        sw.js -> /sw.js                 ✓
/session/abc        -> base /,        sw.js -> /sw.js                 ✓
/session/           -> base /,        sw.js -> /sw.js                 ✓
/hermes/            -> base /hermes/, sw.js -> /hermes/sw.js          ✓ subpath
/hermes/session/x   -> base /hermes/, sw.js -> /hermes/sw.js          ✓ subpath

Relative sw.js resolves correctly in every case, including subpath mounts. The PR description's premise — "On session pages (/session/abc), the browser requests /session/sw.js" — is incorrect. The base-href script computes <base href="<origin>/"> for any path with the /session/ marker (because path.slice(0, i+1) with i=0 returns /), so a relative sw.js resolves to <origin>/sw.js, not <origin>/session/sw.js.

What the change actually breaks. Behind a reverse proxy at /hermes/, the absolute form /sw.js requests <origin>/sw.js — outside the mount root. The upstream proxy 404s before reaching the webui. The relative form correctly requests <origin>/hermes/sw.js, which the proxy forwards.

What I'd recommend. Revert this one line back to relative. The other root cause for issue #1481 ("/sw.js whitelisted but no service worker file exists") is already addressed in master — static/sw.js was added in PR #920 long ago and the route handler at api/routes.py:1173-1194 serves it with a Service-Worker-Allowed: / header. There's nothing left to fix in this code path; the original relative sw.js already works for both root and subpath mounts.

A regression test pinning the relative form would be a nice add (so a future "absolute is cleaner" rewrite can't quietly regress subpath users again).

Fix 2 — tool-card args readability ✅ partial but correct

static/style.css:1700-1701:

-.tool-arg-key{color:var(--blue);font-family:'SF Mono',ui-monospace,monospace;font-size:var(--font-size-xs);}
-.tool-arg-val{color:var(--muted);font-family:'SF Mono',ui-monospace,monospace;font-size:var(--font-size-xs);word-break:break-all;}
+.tool-arg-key{color:var(--blue);font-family:'SF Mono',ui-monospace,monospace;font-size:var(--font-size-xs);display:block;margin-bottom:2px;}
+.tool-arg-val{color:var(--muted);font-family:'SF Mono',ui-monospace,monospace;font-size:var(--font-size-xs);white-space:pre-wrap;word-break:break-word;display:block;overflow-x:auto;}

This correctly addresses the primary issue from #1484: white-space: pre-wrap preserves newlines and indentation, word-break: break-word wraps on word boundaries instead of mid-character. ✅

Two follow-ups left on the table from issue #1484 (non-blocking, can ship in a follow-up):

• .tool-arg-val has no max-height + overflow-y — for very large args (e.g. a 100-line code arg to execute_code), the args section will just expand the whole card. The issue suggested max-height: 360px + overflow-y: auto so very long args stay scrollable.
• .tool-card-result pre max-height stays at 240px — the issue noted this was tight inside a 600px parent and suggested 480px. Not addressed by this PR.

Strictly an improvement over master. Worth merging the partial fix.

Fix 3 — CLI session rename revert ✅ pragmatic, with one perf note

api/models.py:1043-1052 adds:

try:
    _webui_meta = Session.load_metadata_only(sid)
    if _webui_meta and getattr(_webui_meta, 'title', None):
        _title = _webui_meta.title
except Exception:
    pass

Traced: after a CLI session is imported (creates <sid>.json) and then renamed via /api/session/rename (api/routes.py:1978-1990), the JSON file's title field is updated. On the next refresh, _project_agent_session_rows (api/agent_sessions.py:175-203) merges the chain head's title into the row but exposes the tip's id. Without this fix, the rename is silently overwritten. The PR's lookup Session.load_metadata_only(sid=tip_id) correctly finds the JSON the rename API wrote to.

The issue's suggested fix (walk parent_session_id upward to find the lineage root) is more thorough — it would handle the case where a rename happens before compression and then compression creates a new tip. The PR doesn't cover that scenario, but the repro in the issue (compression first, then rename) is what users actually hit, and the PR fixes that. ✅

Performance note (non-blocking). This adds one Session.load_metadata_only(sid) call per CLI row in _project_agent_session_rows. For a user with 200+ CLI sessions, that's 200 file existence checks + small reads on every sidebar refresh. load_metadata_only only reads the first ~16KB so it's fast (~5ms total in practice), and the existing function already does cron job-name lookups for cron rows (api/models.py:1026-1042) — comparable cost. Not a problem in practice; flagging for awareness in case sidebar latency ever becomes a concern.

Fix 4 — scroll pinning programmatic-vs-user disambiguation ✅ partial but correct

static/ui.js:1180-1196,1399-1410:

let _programmaticScroll=false;
// ... in scroll listener:
if(_programmaticScroll) return; // ignore scrolls we triggered ourselves
// ... in scrollIfPinned() / scrollToBottom():
_programmaticScroll=true; el.scrollTop=el.scrollHeight;
setTimeout(()=>{_programmaticScroll=false;},0);

This is one of the two suggestions from @berkinduz in #1469 — decouple programmatic from user scrolls. The fix is correct: scroll events fire async (microtask after scrollTop=...), and setTimeout(0) queues the flag clear in the next macrotask, so the listener correctly bails on the programmatic scroll while still firing for genuine user scrolls.

Not addressed (per @berkinduz's other suggestions):

• Drive unpin from user intent (wheel deltaY < 0, touchmove, keydown ArrowUp/PageUp/Home). The 250px position threshold is still in place, so slow trackpad scrolling within an active stream may still re-snap before the user accumulates enough delta to cross the threshold.
• Re-pin only when user reaches actual bottom (<8px tolerance). Still uses 250px.

The PR is strictly an improvement (no more programmatic-scroll false-positive re-pinning), but the slow-trackpad case @berkinduz described in the issue may still reproduce. Worth a follow-up that adopts intent-based unpinning. Not blocking for this PR.

Other audit — things that are correct

• ✅ No XSS surface in any of the 4 fixes; CSS-only and DOM-only changes.
• ✅ Session.load_metadata_only(sid) validates the sid shape (api/models.py:427) so no path traversal even if state.db somehow contained a hostile id.
• ✅ Cross-tool: webui-only changes; no config.yaml writes, no agent-side surface.
• ✅ Tests pass: 3813 passed, 55 skipped, 3 xpassed, 0 failures on full suite.
• ✅ CI green on 3.11/3.12/3.13.

Edge-case trace

Recommendation

❌ Request changes. Three of the four fixes (#1484, #1486, #1469) are correct partial improvements and worth merging. The /sw.js change (#1481) is the blocker — it makes the path absolute in violation of the documented "all static paths must stay relative" convention, and breaks reverse-proxy installs at a subpath. Reverting that one line plus adding a regression test pinning the relative form would unblock this PR.

If you'd prefer to drop #1481 entirely and ship the other three: that also works — issue #1481's premise (no sw.js file exists) is already addressed in master by the existing static/sw.js + the route handler at api/routes.py:1173-1194 that serves it. There's nothing left to fix in the path code; the original relative sw.js?v=... works correctly for both root and subpath mounts.

Happy to do a fast follow-up review when the sw.js portion is reverted or removed.