diff --git a/CHANGELOG.md b/CHANGELOG.md index 42919130..6db39afd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,21 @@ ## [Unreleased] +## [v0.51.135] — 2026-05-25 — Release DG (stage-batch17 — 9-PR small-fix batch) + +### Added + +- Added a proposed canonical session resolution RFC covering URL routes, query parameters, localStorage, sidebar rows, and compression-lineage IDs so future session-routing fixes have one review contract. + +### Fixed + +- Browser session links that use the API-style `?session_id=` query parameter now open the requested conversation instead of falling back to the last locally stored session. +- Gateway status now treats existing messaging-session metadata as configured when `gateway.status` is unavailable, avoiding a misleading "Gateway not configured" warning for multi-container deployments with active gateway sessions. +- Session sidebar Archive/Delete menu actions now repaint from local sidebar state immediately after the server confirms the mutation, instead of waiting for the full `/api/sessions` refresh before the row disappears. +- Clarification dialogs now reserve transcript space while open or collapsed, so the question prompt no longer covers the assistant text needed to answer it. +- Chat uploads now send the absolute server-side path for image attachments in the agent text context, restoring immediate tool access (e.g. `vision_analyze`) to files uploaded in the current turn. +- Pending uploaded-file user turns no longer double-render when both the optimistic bubble and the server's pending-message hydration produce the same `[Attached files: ...]` suffix. + ## [v0.51.134] — 2026-05-25 — Release DF (stage-batch16 — single-PR Windows path defaults) ### Fixed diff --git a/api/agent_health.py b/api/agent_health.py index 2ee564e5..897827ab 100644 --- a/api/agent_health.py +++ b/api/agent_health.py @@ -174,10 +174,24 @@ def _gateway_root_pid_path() -> Path | None: Gateway runtime files are root-level singletons. A profile-scoped WebUI process may have HERMES_HOME=/profiles/, but gateway.pid, gateway.lock, and gateway_state.json still live under . + + When the root-level gateway.pid is absent (profile-scoped gateway + deployments write it under /profiles//), fall back to the + active profile's directory so the gateway is detected correctly. """ try: from hermes_constants import get_default_hermes_root - return get_default_hermes_root() / _GATEWAY_PID_FILE + root_pid = get_default_hermes_root() / _GATEWAY_PID_FILE + if root_pid.exists(): + return root_pid + try: + from api.profiles import get_active_hermes_home + profile_pid = Path(get_active_hermes_home()) / _GATEWAY_PID_FILE + if profile_pid.exists(): + return profile_pid + except Exception: + pass + return root_pid except Exception: return None diff --git a/api/routes.py b/api/routes.py index 8140f3f9..bbce9aba 100644 --- a/api/routes.py +++ b/api/routes.py @@ -4754,7 +4754,7 @@ def handle_get(handler, parsed) -> bool: configured = True else: # alive is None → gateway not configured / unavailable running = bool(identity_map) - configured = False + configured = bool(identity_map) platforms_set: set[str] = set() for meta in identity_map.values(): diff --git a/docs/CONTRACTS.md b/docs/CONTRACTS.md index 44649946..777a7436 100644 --- a/docs/CONTRACTS.md +++ b/docs/CONTRACTS.md @@ -27,6 +27,11 @@ does not change runtime behavior, maintainer policy, bot behavior, or CI gates. model-context reconstruction, compression, UI scene/cache, and sidebar metadata repairs. Start here for narrow fixes that keep the existing WebUI execution path. +- [`docs/rfcs/canonical-session-resolution.md`](rfcs/canonical-session-resolution.md): + proposed contract for resolving URL routes, query parameters, localStorage, + sidebar rows, and compression-lineage IDs to one canonical visible session + target. Start here for session routing, boot restore, stale parent, or + compression-tip selection changes. - [`docs/rfcs/hermes-run-adapter-contract.md`](rfcs/hermes-run-adapter-contract.md): proposed event/control contract, runtime-state ownership matrix, acceptance-test catalog, and reversible migration gates for moving WebUI diff --git a/docs/images/pr-2919-clarify-after-desktop.png b/docs/images/pr-2919-clarify-after-desktop.png new file mode 100644 index 00000000..344ba3dc Binary files /dev/null and b/docs/images/pr-2919-clarify-after-desktop.png differ diff --git a/docs/images/pr-2919-clarify-before-desktop.png b/docs/images/pr-2919-clarify-before-desktop.png new file mode 100644 index 00000000..4ad42912 Binary files /dev/null and b/docs/images/pr-2919-clarify-before-desktop.png differ diff --git a/docs/rfcs/README.md b/docs/rfcs/README.md index 58b9291c..9cd72388 100644 --- a/docs/rfcs/README.md +++ b/docs/rfcs/README.md @@ -46,5 +46,8 @@ First-time contributor RFCs should be discussed in an issue before opening a PR. — #2361 consistency rules for keeping transcript, model context, live streams, replay, compression, and session metadata coherent during active and recovered WebUI runs. +- [`canonical-session-resolution.md`](canonical-session-resolution.md) — #2361 + focused contract for resolving URL, query parameter, localStorage, sidebar, + and compression-lineage session IDs to one canonical visible chat target. - [`turn-journal.md`](turn-journal.md) — Crash-safe WebUI turn journal for recovering interrupted chat submissions. diff --git a/docs/rfcs/canonical-session-resolution.md b/docs/rfcs/canonical-session-resolution.md new file mode 100644 index 00000000..5f1a416c --- /dev/null +++ b/docs/rfcs/canonical-session-resolution.md @@ -0,0 +1,124 @@ +# Canonical Session Resolution Contract + +- **Status:** Proposed +- **Author:** @ai-ag2026 +- **Created:** 2026-05-25 +- **Tracking issue:** [#2361](https://github.com/nesquena/hermes-webui/issues/2361) +- **Related architecture:** [#1925](https://github.com/nesquena/hermes-webui/issues/1925), [`webui-run-state-consistency-contract.md`](webui-run-state-consistency-contract.md) + +## Problem + +WebUI can reach the same conversation through several browser-facing entrypoints: + +- a URL route such as `/session/`, +- a query parameter such as `?session=` or `?session_id=`, +- the browser's `localStorage` active-session value, +- sidebar rows built from `/api/sessions`, +- direct session open actions from links, search, or imported session lists, +- browser boot restore after reload, auth redirect, or PWA resume. + +After automatic compression, those entrypoints can point at different rows in one +logical conversation lineage. A pre-compression parent snapshot can remain a +valid archived session while the user-facing conversation tip has moved to a +newer continuation. If each caller resolves IDs independently, the UI can appear +to lose the session, reopen an old one-message snapshot, duplicate sidebar rows, +or prefer the wrong transcript even though durable data is still present. + +This contract defines the expected resolution semantics for those entrypoints. It +is intentionally narrower than the run adapter RFC: this is about choosing the +correct visible session target, not moving execution ownership. + +## Goals + +- Define one canonical browser-facing resolution concept for sessions and + compression lineage. +- Make URL, query parameter, localStorage, sidebar, and direct-open behavior use + the same mental model. +- Preserve archived parent snapshots without letting them become the default + active target when a continuation exists. +- Give reviewers a small checklist for future session-routing, sidebar, and + compression-lineage changes. + +## Non-goals + +- Do not delete archived `pre_compression_snapshot` rows. +- Do not merge or rewrite session files as part of this contract. +- Do not replace state.db/session sidecar reconciliation. +- Do not require a new backend endpoint before narrow frontend guards can land. +- Do not change explicit history browsing when the user deliberately opens an + archived snapshot as a record. + +## Terms + +| Term | Meaning | +|---|---| +| Requested session ID | The ID supplied by route, query parameter, localStorage, sidebar click, or direct session open. | +| Canonical visible session | The session row WebUI should display by default for normal chat navigation. | +| `canonical_visible_session_id` | Proposed field/name for an API or helper output that identifies the canonical visible session. | +| Compression snapshot | A preserved archived parent row with `pre_compression_snapshot` set. | +| Continuation session | The active child/tip created after compression, usually represented by `continuation_session_id`, `_lineage_tip_id`, or newer lineage metadata. | +| Lineage relation | Links such as `parent_session_id`, `_lineage_root_id`, `_lineage_tip_id`, and `_compression_segment_count` that connect rows belonging to one logical conversation. | + +## Resolution Rules + +1. **Directly valid non-snapshot IDs stay stable.** If the requested session ID + exists and is not a `pre_compression_snapshot`, it should normally resolve to + itself. +2. **Snapshot parents defer to visible continuation tips.** If the requested + session ID is a `pre_compression_snapshot` and the session list has a newer + non-snapshot continuation in the same lineage, normal chat navigation should + resolve to that continuation as the `canonical_visible_session_id`. +3. **Explicit archive/history inspection remains possible.** A future UI affordance + may intentionally open a snapshot as a historical record, but that should be a + distinct mode from ordinary boot restore, URL restore, or sidebar continuation. +4. **Local browser state is advisory.** `localStorage` may remember the last active + ID, but browser boot restore must treat it as a requested session ID and still + run canonical resolution before rendering. +5. **Query aliases share the same resolver.** `?session=...`, `?session_id=...`, + and `/session/...` should feed the same requested-ID path instead of carrying + separate precedence rules. +6. **Sidebar collapse and session loading agree.** The row chosen as the visible + representative for a lineage should match the target opened by `loadSession()` + for that lineage during ordinary navigation. +7. **404 self-heal is separate from lineage resolution.** Missing/deleted sessions + should still use the stale-route recovery path. A present archived parent with + a live continuation is not a 404; it is a canonicalization problem. + +## Entry Point Matrix + +| Entry point | Input | Expected resolution | +|---|---|---| +| URL route | `/session/` | Treat `` as requested; resolve to canonical visible session before ordinary render. | +| Query parameter | `?session=` or `?session_id=` | Same as URL route. Query spelling must not change the target semantics. | +| localStorage | last active session ID | Advisory requested ID during browser boot restore; canonicalize before render. | +| Sidebar click | visible row ID or lineage representative | Open the same canonical visible session that the row represents. | +| Direct session open | programmatic call/search/import link | Use the shared requested-ID resolver unless the caller explicitly opts into archive inspection. | +| Browser boot restore | URL and/or localStorage state after reload/auth/PWA resume | Prefer explicit URL/query input, then localStorage, then canonicalize the requested ID. | + +## Review Checklist + +For PRs that touch session routing, compression lineage, sidebar collapse, boot +restore, direct session open, or URL parsing, answer: + +- Which entrypoints provide the requested session ID? +- Does the code path accept both route and query parameter forms where relevant? +- Does localStorage go through the same canonicalization path as URL restore? +- Can a `pre_compression_snapshot` become the default active chat when a + non-snapshot `continuation_session_id` / `_lineage_tip_id` exists? +- Do sidebar collapse and `loadSession()` pick the same visible representative? +- Is missing-session 404 recovery kept distinct from present-but-archived lineage + canonicalization? +- What regression proves route, query parameter, localStorage, and sidebar paths + agree for compressed lineage rows? + +## Rollout Plan + +1. Document this proposed contract and link it from the public contract index. +2. Keep narrow bugfixes small while referencing the relevant rule they preserve. +3. Add shared frontend helper coverage for URL/query/localStorage/sidebar + requested-ID inputs. +4. If backend session APIs later expose `canonical_visible_session_id`, make the + frontend resolver prefer the backend value while preserving client fallback for + older WebUI servers. +5. If #1925 moves execution/session ownership behind an adapter, carry this + contract forward as an adapter-facing session-navigation invariant. diff --git a/static/messages.js b/static/messages.js index f10b5d9d..09bae5ca 100644 --- a/static/messages.js +++ b/static/messages.js @@ -433,7 +433,7 @@ async function send(){ setComposerStatus(''); const uploadedNames=uploaded.map(u=>u.name||u); - const uploadedPaths=uploaded.map(u=>u&&u.is_image?(u.name||u.filename||u):(u.path||u.name||u)); + const uploadedPaths=uploaded.map(u=>u&&u.path?u.path:(u&&u.name?u.name:(u&&u.filename?u.filename:u))); let msgText=text; if(uploaded.length&&!msgText)msgText=`I've uploaded ${uploaded.length} file(s): ${uploadedPaths.join(', ')}`; else if(uploaded.length)msgText=`${text}\n\n[Attached files: ${uploadedPaths.join(', ')}]`; @@ -2677,12 +2677,63 @@ function _syncClarifyCollapseButton(card) { collapse.title = label; } +let _clarifyResizeListenerReady = false; + +function _clarifyMessagesNearBottom(messages) { + if (!messages) return false; + return messages.scrollHeight - messages.scrollTop - messages.clientHeight < 150; +} + +function _syncClarifyTranscriptSpace(card, opts) { + opts = opts || {}; + const messages = $("messages"); + if (!messages) return; + const wasNearBottom = _clarifyMessagesNearBottom(messages); + if (!card || !card.classList.contains("visible")) { + messages.classList.remove("clarify-open"); + messages.classList.remove("clarify-collapsed"); + messages.style.removeProperty("--clarify-card-height"); + messages.style.removeProperty("--clarify-dock-height"); + if (wasNearBottom && typeof scrollToBottom === "function" && typeof requestAnimationFrame === "function") { + requestAnimationFrame(scrollToBottom); + } + return; + } + const collapsed = card.classList.contains("collapsed"); + messages.classList.add("clarify-open"); + messages.classList.toggle("clarify-collapsed", collapsed); + const measure = () => { + if (!card.classList.contains("visible")) return; + const target = collapsed ? card : (card.querySelector(".clarify-inner") || card); + const h = target && target.getBoundingClientRect().height; + if (h > 0) { + messages.style.setProperty(collapsed ? "--clarify-dock-height" : "--clarify-card-height", Math.ceil(h + 24) + "px"); + } + if (wasNearBottom && typeof scrollToBottom === "function") scrollToBottom(); + }; + if (opts.immediate) measure(); + if (typeof requestAnimationFrame === "function") requestAnimationFrame(measure); + setTimeout(measure, 420); +} + +function _ensureClarifyResizeListener() { + if (_clarifyResizeListenerReady || typeof window === "undefined") return; + _clarifyResizeListenerReady = true; + window.addEventListener("resize", () => { + const card = $("clarifyCard"); + if (card && card.classList.contains("visible")) { + _syncClarifyTranscriptSpace(card, {immediate: true}); + } + }, {passive: true}); +} + function toggleClarifyCardCollapsed(forceCollapsed) { const card = $("clarifyCard"); if (!card) return; const collapsed = typeof forceCollapsed === "boolean" ? forceCollapsed : !card.classList.contains("collapsed"); card.classList.toggle("collapsed", collapsed); _syncClarifyCollapseButton(card); + _syncClarifyTranscriptSpace(card, {immediate: true}); } function _clearClarifyHideTimer() { @@ -2797,6 +2848,7 @@ function hideClarifyCard(force=false, reason="dismissed") { _clarifySessionId = null; _resetClarifyCardState(); card.classList.remove("visible"); + _syncClarifyTranscriptSpace(null); if (typeof unlockComposerForClarify === "function") unlockComposerForClarify(); $("clarifyQuestion").textContent = ""; $("clarifyChoices").innerHTML = ""; @@ -2911,8 +2963,10 @@ function showClarifyCard(pending) { lockComposerForClarify(question ? `Clarification needed: ${question}` : "Clarification needed"); } _clarifySetControlsDisabled(false, false); + _ensureClarifyResizeListener(); card.classList.add("visible"); _syncClarifyCollapseButton(card); + _syncClarifyTranscriptSpace(card, {immediate: true}); if (typeof applyLocaleToDOM === "function") applyLocaleToDOM(); // Move focus to clarify input synchronously (not in setTimeout) and // only if the user wasn't mid-type in the composer textarea. diff --git a/static/sessions.js b/static/sessions.js index 83d8c5c4..bd80d9a8 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -647,6 +647,7 @@ async function loadSession(sid){ if(!Array.isArray(messages)) return false; const pendingMsg=typeof getPendingSessionMessage==='function'?getPendingSessionMessage(session,messages):null; if(!pendingMsg) return false; + if(messages.some(existing=>_sameTranscriptMessage(existing,pendingMsg))) return false; const liveAssistantIdx=messages.findIndex(m=>m&&m.role==='assistant'&&m._live); if(liveAssistantIdx>=0) messages.splice(liveAssistantIdx,0,pendingMsg); else messages.push(pendingMsg); @@ -1544,6 +1545,24 @@ function _sessionArchiveToast(response, session){ function _sessionDeleteDescription(session){ return session&&session.worktree_path?t('session_delete_worktree_desc'):t('session_delete_desc'); } +function _optimisticallyArchiveSessionInList(sid, archived){ + if(!sid||!Array.isArray(_allSessions)) return; + let changed=false; + _allSessions=_allSessions.map(s=>{ + if(!s||s.session_id!==sid) return s; + changed=true; + return {...s,archived:!!archived}; + }); + if(changed) renderSessionListFromCache(); +} +function _optimisticallyRemoveSessionFromList(sid){ + if(!sid||!Array.isArray(_allSessions)) return; + const before=_allSessions.length; + _allSessions=_allSessions.filter(s=>!s||s.session_id!==sid); + if(_selectedSessions&&_selectedSessions.has(sid)) _selectedSessions.delete(sid); + if(typeof _dropStaleOptimisticSessionRow==='function') _dropStaleOptimisticSessionRow(sid); + if(_allSessions.length!==before) renderSessionListFromCache(); +} function _sessionIdFromLocation(){ if(typeof window==='undefined'||!window.location) return null; @@ -1556,7 +1575,7 @@ function _sessionIdFromLocation(){ } try{ const qs=new URLSearchParams(window.location.search||''); - return qs.get('session')||null; + return qs.get('session')||qs.get('session_id')||null; }catch(_e){return null;} } function _sessionUrlForSid(sid){ @@ -1866,9 +1885,10 @@ function _openSessionActionMenu(session, anchorEl){ closeSessionActionMenu(); try{ const response=await api('/api/session/archive',{method:'POST',body:JSON.stringify({session_id:session.session_id,archived:!session.archived})}); + _optimisticallyArchiveSessionInList(session.session_id,!session.archived); session.archived=!session.archived; if(S.session&&S.session.session_id===session.session_id) S.session.archived=session.archived; - await renderSessionList(); + void renderSessionList(); showToast(session.archived?_sessionArchiveToast(response,session):t('session_restored')); }catch(err){showToast(t('session_archive_failed')+err.message);} } @@ -1882,9 +1902,10 @@ function _openSessionActionMenu(session, anchorEl){ closeSessionActionMenu(); try{ await api('/api/session/archive',{method:'POST',body:JSON.stringify({session_id:session.session_id,archived:true})}); + _optimisticallyArchiveSessionInList(session.session_id,true); session.archived=true; if(S.session&&S.session.session_id===session.session_id) S.session.archived=true; - await renderSessionList(); + void renderSessionList(); showToast(t('session_hidden')); }catch(err){showToast(t('session_archive_failed')+err.message);} } @@ -3874,6 +3895,7 @@ async function deleteSession(sid){ let response=null; try{ response=await api('/api/session/delete',{method:'POST',body:JSON.stringify({session_id:sid})}); + _optimisticallyRemoveSessionFromList(sid); _clearHandoffStorageForSession(sid); }catch(e){setStatus(`Delete failed: ${e.message}`);return;} if(S.session&&S.session.session_id===sid){ @@ -3894,7 +3916,7 @@ async function deleteSession(sid){ } } showToast(_sessionResponseRetainsWorktree(response,session)?t('session_deleted_worktree'):t('session_deleted')); - await renderSessionList(); + void renderSessionList(); } // ── Project helpers ───────────────────────────────────────────────────── diff --git a/static/style.css b/static/style.css index b658d1fa..7ff52d11 100644 --- a/static/style.css +++ b/static/style.css @@ -984,6 +984,8 @@ .messages.terminal-open{padding-bottom:var(--terminal-card-height,320px);scroll-padding-bottom:var(--terminal-card-height,320px);transition:padding-bottom .26s cubic-bezier(.2,.8,.2,1);} .messages.terminal-collapsed{padding-bottom:var(--terminal-dock-height,72px);scroll-padding-bottom:var(--terminal-dock-height,72px);transition:padding-bottom .22s cubic-bezier(.2,.8,.2,1);} .messages.handoff-dock-visible{padding-bottom:var(--handoff-dock-height,72px);scroll-padding-bottom:var(--handoff-dock-height,72px);transition:padding-bottom .22s cubic-bezier(.2,.8,.2,1);} + .messages.clarify-open{padding-bottom:var(--clarify-card-height,320px);scroll-padding-bottom:var(--clarify-card-height,320px);transition:padding-bottom .22s cubic-bezier(.2,.8,.2,1);} + .messages.clarify-collapsed{padding-bottom:var(--clarify-dock-height,72px);scroll-padding-bottom:var(--clarify-dock-height,72px);} .messages.terminal-expanding-from-dock{transition:none!important;} .queue-card-inner{background:var(--surface);border:1px solid var(--border);border-bottom:none;border-radius:14px 14px 0 0;contain:paint;transform:translateY(100%);opacity:0;transition:transform .35s cubic-bezier(.32,.72,.16,1),opacity .2s ease;overflow:hidden;max-height:240px;overflow-y:auto;padding-bottom:4px;} .queue-card.visible .queue-card-inner{transform:translateY(0);opacity:1;} diff --git a/tests/test_agent_health_pid_path_fallback.py b/tests/test_agent_health_pid_path_fallback.py new file mode 100644 index 00000000..27485009 --- /dev/null +++ b/tests/test_agent_health_pid_path_fallback.py @@ -0,0 +1,112 @@ +"""Regression coverage for _gateway_root_pid_path() profile-scoped fallback. + +Before the fix, _gateway_root_pid_path() unconditionally returned +/gateway.pid. Profile-scoped gateways (running via +``gateway run --profile `` or with ``active_profile`` set) write +their PID file under /profiles//gateway.pid instead of +the root, so the root-level file never existed. The WebUI's +build_agent_health_payload() therefore always received a non-existent +pid_path, fell through to the stale root-level gateway_state.json, and +returned alive=None — causing the cron page to display "Gateway not +configured" even though the gateway was running. + +Fix: when the root-level gateway.pid is absent, _gateway_root_pid_path() +now falls back to the active profile's directory. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +pytest.importorskip("hermes_constants", reason="hermes-agent not installed") + + +def _call(monkeypatch, root: Path, profile_dir: Path | None = None) -> Path | None: + """Call _gateway_root_pid_path() with mocked filesystem roots.""" + import hermes_constants + import api.profiles as profiles + + monkeypatch.setattr(hermes_constants, "get_default_hermes_root", lambda: root) + if profile_dir is not None: + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: str(profile_dir)) + + from api.agent_health import _gateway_root_pid_path + return _gateway_root_pid_path() + + +# ── core behaviour ──────────────────────────────────────────────────────────── + +def test_returns_root_pid_when_root_level_file_exists(tmp_path, monkeypatch): + """Root-level gateway.pid present → return it (original behaviour unchanged).""" + root = tmp_path / "hermes" + root.mkdir() + root_pid = root / "gateway.pid" + root_pid.write_text("1234") + profile_dir = root / "profiles" / "other" + profile_dir.mkdir(parents=True) + (profile_dir / "gateway.pid").write_text("9999") + + result = _call(monkeypatch, root=root, profile_dir=profile_dir) + assert result == root_pid + + +def test_falls_back_to_profile_pid_when_root_absent(tmp_path, monkeypatch): + """Root gateway.pid absent + profile-level exists → return profile path.""" + root = tmp_path / "hermes" + root.mkdir() + # root-level gateway.pid intentionally not created + profile_dir = root / "profiles" / "safeline" + profile_dir.mkdir(parents=True) + profile_pid = profile_dir / "gateway.pid" + profile_pid.write_text("5678") + + result = _call(monkeypatch, root=root, profile_dir=profile_dir) + assert result == profile_pid + + +def test_returns_root_path_when_neither_pid_exists(tmp_path, monkeypatch): + """Neither root nor profile gateway.pid exists → return root path (graceful).""" + root = tmp_path / "hermes" + root.mkdir() + profile_dir = root / "profiles" / "empty" + profile_dir.mkdir(parents=True) + # no gateway.pid created anywhere + + result = _call(monkeypatch, root=root, profile_dir=profile_dir) + assert result == root / "gateway.pid" + + +def test_returns_root_path_when_profile_lookup_raises(tmp_path, monkeypatch): + """get_active_hermes_home() raising must be caught; root path returned.""" + root = tmp_path / "hermes" + root.mkdir() + + import hermes_constants + import api.profiles as profiles + + monkeypatch.setattr(hermes_constants, "get_default_hermes_root", lambda: root) + + def _raise(): + raise RuntimeError("profile resolution failed") + + monkeypatch.setattr(profiles, "get_active_hermes_home", _raise) + + from api.agent_health import _gateway_root_pid_path + result = _gateway_root_pid_path() + assert result == root / "gateway.pid" + + +def test_root_takes_priority_over_profile_when_both_exist(tmp_path, monkeypatch): + """Root gateway.pid present even when profile pid also exists → root wins.""" + root = tmp_path / "hermes" + root.mkdir() + root_pid = root / "gateway.pid" + root_pid.write_text("1111") + profile_dir = root / "profiles" / "safeline" + profile_dir.mkdir(parents=True) + (profile_dir / "gateway.pid").write_text("2222") + + result = _call(monkeypatch, root=root, profile_dir=profile_dir) + assert result == root_pid diff --git a/tests/test_canonical_session_resolution_rfc.py b/tests/test_canonical_session_resolution_rfc.py new file mode 100644 index 00000000..f3edc687 --- /dev/null +++ b/tests/test_canonical_session_resolution_rfc.py @@ -0,0 +1,37 @@ +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +RFC = ROOT / "docs" / "rfcs" / "canonical-session-resolution.md" +RFC_INDEX = ROOT / "docs" / "rfcs" / "README.md" +CONTRACTS = ROOT / "docs" / "CONTRACTS.md" + + +def test_canonical_session_resolution_rfc_is_indexed(): + assert RFC.exists(), "canonical session resolution RFC must exist" + + rel = "docs/rfcs/canonical-session-resolution.md" + rfc_index = RFC_INDEX.read_text(encoding="utf-8") + contracts = CONTRACTS.read_text(encoding="utf-8") + + assert "canonical-session-resolution.md" in rfc_index + assert rel in contracts + + +def test_canonical_session_resolution_contract_names_entrypoints_and_outputs(): + text = RFC.read_text(encoding="utf-8") + + required_terms = [ + "URL route", + "query parameter", + "localStorage", + "sidebar", + "pre_compression_snapshot", + "canonical_visible_session_id", + "continuation_session_id", + "parent_session_id", + "direct session open", + "browser boot restore", + ] + + missing = [term for term in required_terms if term not in text] + assert missing == [] diff --git a/tests/test_chat_upload_attachment_paths.py b/tests/test_chat_upload_attachment_paths.py new file mode 100644 index 00000000..6e0fba38 --- /dev/null +++ b/tests/test_chat_upload_attachment_paths.py @@ -0,0 +1,21 @@ +"""Regression coverage for WebUI chat upload path handoff.""" +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +MESSAGES_JS = ROOT / "static" / "messages.js" + + +def test_image_uploads_use_server_path_in_attached_files_context(): + """The agent text context must include real uploaded paths for images. + + /api/upload returns an absolute attachment path. The browser also sends the + structured attachment payload to /api/chat/start, but text/tool-mode agents + still rely on the literal ``[Attached files: ...]`` suffix. Images must not + be downgraded to bare filenames there, otherwise tools like vision_analyze + cannot open the uploaded file immediately. + """ + src = MESSAGES_JS.read_text(encoding="utf-8") + + assert "uploadedPaths=uploaded.map(u=>u&&u.is_image?" not in src + assert "uploadedPaths=uploaded.map(u=>u&&u.path?u.path" in src diff --git a/tests/test_gateway_status_agent_health.py b/tests/test_gateway_status_agent_health.py index 843f7cd5..66a70f45 100644 --- a/tests/test_gateway_status_agent_health.py +++ b/tests/test_gateway_status_agent_health.py @@ -124,7 +124,8 @@ def test_gateway_status_running_true_and_platforms_when_agent_health_alive_and_s def test_gateway_status_alive_none_falls_back_to_identity_map_heuristic(monkeypatch): """When alive=None (not configured) but sessions exist, running reflects identity_map. - configured=false tells the frontend to show 'not configured' state.""" + configured=true because sessions metadata proves a gateway is configured + even if gateway.status cannot be imported in this WebUI process.""" from api import routes monkeypatch.setattr( @@ -144,8 +145,8 @@ def test_gateway_status_alive_none_falls_back_to_identity_map_heuristic(monkeypa result = handler.get_json() # Fallback to identity_map: sessions exist → running=true assert result["running"] is True - # But configured=false because alive was None (no gateway metadata) - assert result["configured"] is False + # Existing gateway sessions prove the gateway has been configured. + assert result["configured"] is True def test_gateway_status_handles_corrupted_sessions_json(monkeypatch): @@ -244,4 +245,4 @@ def test_gateway_status_last_active_empty_when_alive_and_no_sessions_path(monkey assert result["configured"] is True # In test context, sessions_path won't exist (no real filesystem), # so last_active must be empty. - assert result["last_active"] == "" \ No newline at end of file + assert result["last_active"] == "" diff --git a/tests/test_issue2762_state_sync_profile_kwarg.py b/tests/test_issue2762_state_sync_profile_kwarg.py index 577e289f..487941e1 100644 --- a/tests/test_issue2762_state_sync_profile_kwarg.py +++ b/tests/test_issue2762_state_sync_profile_kwarg.py @@ -25,6 +25,7 @@ from __future__ import annotations import sys import sqlite3 +import threading from pathlib import Path import pytest @@ -96,6 +97,72 @@ def _read_session(db_path: Path, session_id: str): conn.close() +def _make_message_state_db(path: Path, session_id: str, message_count: int, label: str): + conn = sqlite3.connect(path) + try: + conn.execute( + "CREATE TABLE sessions (id TEXT PRIMARY KEY, source TEXT, title TEXT, model TEXT, started_at REAL, message_count INTEGER)" + ) + conn.execute( + "CREATE TABLE messages (id INTEGER PRIMARY KEY AUTOINCREMENT, session_id TEXT, role TEXT, content TEXT, timestamp REAL)" + ) + conn.execute( + "INSERT INTO sessions (id, source, title, model, started_at, message_count) VALUES (?, ?, ?, ?, ?, ?)", + (session_id, "webui", label, "test-model", 1000.0, message_count), + ) + for idx in range(message_count): + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, ?, ?, ?)", + ( + session_id, + "user" if idx % 2 == 0 else "assistant", + f"{label} message {idx + 1}", + 1000.0 + idx, + ), + ) + conn.commit() + finally: + conn.close() + + +@pytest.fixture() +def two_profile_message_homes(tmp_path, monkeypatch): + """Minimal multi-profile state.db homes for metadata-only read-path tests.""" + import api.config as config + import api.models as models_mod + import api.profiles as profiles_mod + import api.routes as routes_mod + + hiyuki_home = tmp_path / "hiyuki" + maiko_home = tmp_path / "maiko" + session_dir = tmp_path / "webui-state" / "sessions" + for home in (hiyuki_home, maiko_home, session_dir): + home.mkdir(parents=True) + + sid = "metadata_profile_routing" + _make_message_state_db(hiyuki_home / "state.db", sid, 1, "hiyuki") + _make_message_state_db(maiko_home / "state.db", sid, 3, "maiko") + + def fake_profile_home(name): + if name == "maiko": + return maiko_home + if name == "hiyuki" or name in (None, "", "default"): + return hiyuki_home + raise LookupError(name) + + monkeypatch.setattr(profiles_mod, "get_hermes_home_for_profile", fake_profile_home) + monkeypatch.setattr(profiles_mod, "get_active_hermes_home", lambda: hiyuki_home) + monkeypatch.setattr(models_mod, "_active_state_db_path", lambda: hiyuki_home / "state.db") + monkeypatch.setattr(config, "STATE_DIR", tmp_path / "webui-state", raising=False) + monkeypatch.setattr(config, "SESSION_DIR", session_dir, raising=False) + monkeypatch.setattr(config, "SESSION_INDEX_FILE", session_dir / "_index.json", raising=False) + monkeypatch.setattr(models_mod, "SESSION_DIR", session_dir, raising=False) + monkeypatch.setattr(models_mod, "SESSION_INDEX_FILE", session_dir / "_index.json", raising=False) + monkeypatch.setattr(routes_mod, "_active_state_db_path", lambda: hiyuki_home / "state.db", raising=False) + + return {"sid": sid, "hiyuki": hiyuki_home, "maiko": maiko_home, "session_dir": session_dir} + + def test_get_state_db_honors_explicit_profile_kwarg(two_profile_homes): """_get_state_db(profile='maiko') resolves to maiko's home, NOT the active profile (hiyuki).""" @@ -182,6 +249,101 @@ def test_sync_session_usage_without_profile_kwarg_uses_active(two_profile_homes) "sync_session_usage() without profile= regressed: did not write to active profile's state.db" +def test_metadata_only_summary_reads_explicit_profile_state_db(two_profile_message_homes): + """Metadata-only state.db reads must honor explicit profile=. + + This pins the read-path equivalent of #2762/#2827: if the helper drops the + profile kwarg or stops forwarding it to get_state_db_session_messages(), it + falls back to the active profile (hiyuki) and reports the wrong count. + """ + from api.routes import _metadata_only_message_summary + + sid = two_profile_message_homes["sid"] + + maiko_summary = _metadata_only_message_summary(sid, profile="maiko") + hiyuki_summary = _metadata_only_message_summary(sid) + + assert maiko_summary["message_count"] == 3 + assert hiyuki_summary["message_count"] == 1 + + +def test_metadata_only_summary_honors_profile_from_background_thread(two_profile_message_homes): + """Explicit profile= must work even when TLS active-profile context is absent.""" + from api.routes import _metadata_only_message_summary + + sid = two_profile_message_homes["sid"] + result = {} + + def run(): + result["summary"] = _metadata_only_message_summary(sid, profile="maiko") + + worker = threading.Thread(target=run) + worker.start() + worker.join(timeout=2) + + assert not worker.is_alive() + assert result["summary"]["message_count"] == 3 + + +def test_api_session_metadata_only_passes_session_profile_to_summary( + two_profile_message_homes, monkeypatch +): + """GET /api/session?messages=0 must pass the loaded session profile onward. + + If the call site accidentally invokes _metadata_only_message_summary(sid) + without profile=s.profile, this test reports the active hiyuki count (1) + instead of maiko's count (3). + """ + from urllib.parse import urlparse + from io import BytesIO + + import api.models as models_mod + import api.routes as routes_mod + + sid = two_profile_message_homes["sid"] + session = models_mod.Session( + session_id=sid, + title="Metadata profile routing", + workspace=str(two_profile_message_homes["session_dir"]), + model="test-model", + profile="maiko", + messages=[ + {"role": "user", "content": "maiko message 1", "timestamp": 1000.0} + ], + created_at=1000.0, + updated_at=1001.0, + ) + session.save(touch_updated_at=False) + monkeypatch.setattr(routes_mod, "_lookup_cli_session_metadata", lambda _sid: {}) + + class Handler: + path = f"/api/session?session_id={sid}&messages=0&resolve_model=0" + headers = {} + client_address = ("127.0.0.1", 12345) + + def __init__(self): + self.status = None + self.wfile = BytesIO() + + def send_response(self, status): + self.status = status + + def send_header(self, *_args): + pass + + def end_headers(self): + pass + + def log_message(self, *_args, **_kwargs): + pass + + handler = Handler() + routes_mod.handle_get(handler, urlparse(handler.path)) + + assert handler.status == 200 + assert b'"message_count": 3' in handler.wfile.getvalue() + + def test_unknown_explicit_profile_returns_none_not_falls_back(two_profile_homes): """Copilot review of PR #2827: when ``profile`` is explicit and resolution fails (e.g. typoed profile name, IO error), the diff --git a/tests/test_issue2883_clarify_padding.py b/tests/test_issue2883_clarify_padding.py new file mode 100644 index 00000000..3c289698 --- /dev/null +++ b/tests/test_issue2883_clarify_padding.py @@ -0,0 +1,46 @@ +from pathlib import Path + + +ROOT = Path(__file__).resolve().parent.parent +MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8") +STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8") + + +def _compact(text: str) -> str: + return "".join(text.split()) + + +def test_clarify_card_reserves_transcript_space(): + compact_css = _compact(STYLE_CSS) + + assert ".messages.clarify-open" in STYLE_CSS + assert "padding-bottom:var(--clarify-card-height,320px)" in compact_css + assert "scroll-padding-bottom:var(--clarify-card-height,320px)" in compact_css + + +def test_clarify_collapse_uses_smaller_transcript_space(): + compact_css = _compact(STYLE_CSS) + compact_js = _compact(MESSAGES_JS) + + assert ".messages.clarify-collapsed" in STYLE_CSS + assert "padding-bottom:var(--clarify-dock-height,72px)" in compact_css + assert 'classList.toggle("clarify-collapsed",collapsed)' in compact_js + assert "--clarify-dock-height" in MESSAGES_JS + + +def test_clarify_show_hide_toggle_messages_padding_classes(): + compact_js = _compact(MESSAGES_JS) + + assert "_syncClarifyTranscriptSpace(card,{immediate:true})" in compact_js + assert "_syncClarifyTranscriptSpace(null)" in compact_js + assert 'classList.add("clarify-open")' in MESSAGES_JS + assert 'classList.remove("clarify-open")' in MESSAGES_JS + assert "--clarify-card-height" in MESSAGES_JS + + +def test_clarify_padding_remeasures_on_resize(): + compact_js = _compact(MESSAGES_JS) + + assert "function_ensureClarifyResizeListener()" in compact_js + assert 'window.addEventListener("resize"' in MESSAGES_JS + assert "_ensureClarifyResizeListener();" in MESSAGES_JS diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 0c4d8ab8..d1357eb8 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -650,6 +650,25 @@ def test_loadSession_inflight_merges_tail_with_persisted_transcript(cleanup_test ) + +def test_browser_session_url_accepts_api_session_id_param(cleanup_test_sessions): + """External links using ?session_id=... should open that session in the browser. + + The API endpoint uses `session_id`, while the browser URL historically used + `session`/`/session/`. Auth/cookie bridges and external callers can + legitimately produce `/?session_id=`; ignoring it falls back to stale + localStorage and renders the wrong or empty conversation. + """ + src = (REPO_ROOT / "static/sessions.js").read_text() + start = src.find("function _sessionIdFromLocation") + assert start >= 0, "session URL parser not found" + end = src.find("function _sessionUrlForSid", start) + assert end > start, "session URL parser block end not found" + block = src[start:end] + assert "qs.get('session')" in block or 'qs.get("session")' in block + assert "qs.get('session_id')" in block or 'qs.get("session_id")' in block + + def test_inflight_merge_dedupes_uploaded_user_message(cleanup_test_sessions): """Uploaded-file turns render optimistically before the server stores the final pending text with an `[Attached files: ...]` suffix. The INFLIGHT @@ -665,6 +684,12 @@ def test_inflight_merge_dedupes_uploaded_user_message(cleanup_test_sessions): assert "role==='user'" in src, ( "attached-files normalization should be limited to user turns" ) + pending_idx = src.find("function _mergePendingSessionMessage") + assert pending_idx >= 0, "pending session merge helper not found" + pending_block = src[pending_idx:pending_idx+500] + assert "_sameTranscriptMessage(existing,pendingMsg)" in pending_block, ( + "pending-user merge should reuse transcript identity dedupe before inserting the server pending message" + ) def test_loadSession_inflight_sets_active_stream_before_replaying_live_tool_cards(cleanup_test_sessions): diff --git a/tests/test_session_action_menu_regression.py b/tests/test_session_action_menu_regression.py index e3d36cdc..136b5882 100644 --- a/tests/test_session_action_menu_regression.py +++ b/tests/test_session_action_menu_regression.py @@ -29,3 +29,29 @@ def test_session_list_refresh_does_not_close_open_conversation_actions(): assert "if(_renamingSid) return;" in body assert "if(_sessionActionMenu) return;" in body assert body.index("if(_sessionActionMenu) return;") < body.index("closeSessionActionMenu();") + + +def test_archive_action_repaints_sidebar_before_full_refresh(): + """Archive should hide the row from cached sidebar state before /api/sessions returns.""" + body = _function_block(SESSIONS_JS, "_openSessionActionMenu") + + api_call = "const response=await api('/api/session/archive'" + optimistic = "_optimisticallyArchiveSessionInList(session.session_id,!session.archived);" + full_refresh = "void renderSessionList();" + + assert optimistic in body + assert body.index(api_call) < body.index(optimistic) < body.index(full_refresh) + + +def test_delete_action_repaints_sidebar_before_loading_remaining_sessions(): + """Delete should remove the row locally before loading replacement session data.""" + body = _function_block(SESSIONS_JS, "deleteSession") + + api_call = "response=await api('/api/session/delete'" + optimistic = "_optimisticallyRemoveSessionFromList(sid);" + remaining_fetch = "const remaining=await api('/api/sessions');" + full_refresh = "void renderSessionList();" + + assert optimistic in body + assert body.index(api_call) < body.index(optimistic) < body.index(full_refresh) + assert body.index(optimistic) < body.index(remaining_fetch)