diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index 3ec4a40b3929..abb70b543cbb 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -743,6 +743,17 @@ PLATFORM_HINTS = { "or 'all'). Do not promise the user that a deliver='origin' or " "default-deliver cron job will message them in this session." ), + "desktop": ( + "You are chatting inside the Hermes desktop app — a graphical chat " + "surface, not a terminal. Use markdown freely: it renders with full " + "GitHub flavor (tables, code blocks with syntax highlighting, math " + "via $...$, task lists, blockquote callouts). " + "You can deliver files natively — include MEDIA:/absolute/path/to/file " + "in your response. Images (.png, .jpg, .webp) appear inline, audio and " + "video play inline, and other files arrive as download links. You can " + "also include image URLs in markdown format ![alt](url) and they " + "render inline as photos." + ), "sms": ( "You are communicating via SMS. Keep responses concise and use plain text " "only — no markdown, no formatting. SMS messages are limited to ~1600 " @@ -1127,22 +1138,6 @@ def build_environment_hints() -> str: f"`uname -a && whoami && pwd`." ) - # Hermes desktop GUI — any agent running under the desktop app should know - # it. HERMES_DESKTOP marks the backend powering the chat; HERMES_DESKTOP_TERMINAL - # marks a hermes launched in the embedded terminal pane. Both set by main.cjs. - _truthy = ("1", "true", "yes") - _in_desktop = (os.getenv("HERMES_DESKTOP") or "").strip().lower() in _truthy - _in_desktop_term = (os.getenv("HERMES_DESKTOP_TERMINAL") or "").strip().lower() in _truthy - if _in_desktop or _in_desktop_term: - _desktop_hint = "Runtime surface: you're running inside the Hermes desktop GUI app." - if _in_desktop_term: - _desktop_hint += ( - " You're in its embedded terminal pane, beside the GUI chat — the user can " - "select your output (⌥-drag on macOS, Shift-drag elsewhere) and press " - "⌘/Ctrl+L to send it to the chat composer." - ) - hints.append(_desktop_hint) - if is_wsl(): hints.append(WSL_ENVIRONMENT_HINT) diff --git a/agent/system_prompt.py b/agent/system_prompt.py index b9b26e07abcb..ea33df54a0d0 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -24,6 +24,7 @@ Pure helpers that read the agent's state. AIAgent keeps thin forwarders. from __future__ import annotations import json +import os from typing import Any, Dict, List, Optional from agent.prompt_builder import ( @@ -44,6 +45,7 @@ from agent.prompt_builder import ( drain_truncation_warnings, ) from agent.runtime_cwd import resolve_context_cwd +from utils import is_truthy_value def _ra(): @@ -110,6 +112,36 @@ def _resolve_platform_hint(agent: Any, platform_key: str, default_hint: str) -> return base +_TUI_EMBEDDED_PANE_CLARIFIER = ( + " You're in its embedded terminal pane, beside the GUI chat — the user can " + "select your output (Option-drag on macOS, Shift-drag elsewhere) and press " + "Cmd/Ctrl+L to send it to the chat composer." +) + + +def _tui_embedded_pane_clarifier(hint: str) -> str: + """Append the desktop-embedded-terminal-pane clarifier to a tui hint. + + Triggered by ``HERMES_DESKTOP_TERMINAL=1`` (set by ``main.cjs`` only on the + shell env of the desktop's embedded TUI PTY — never on the chat backend). + This is a runtime-surface qualifier, not a config override, so it lives at + the resolution site rather than inside ``_resolve_platform_hint`` (which + is purely the config-platform_hints override applier). Byte-stable for the + cache: called once per session build, deterministically from env state. + + Idempotent and empty-safe: re-applying on an already-augmented hint is a + no-op, and an empty input returns empty (we never synthesize the + clarifier without its tui framing). + """ + if not hint: + return hint + if _TUI_EMBEDDED_PANE_CLARIFIER in hint: + return hint + if not is_truthy_value(os.getenv("HERMES_DESKTOP_TERMINAL")): + return hint + return hint + _TUI_EMBEDDED_PANE_CLARIFIER + + def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) -> Dict[str, str]: """Assemble the system prompt as three ordered parts. @@ -398,6 +430,8 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) pass _effective_hint = _resolve_platform_hint(agent, platform_key, _default_hint) + if platform_key == "tui" and _effective_hint: + _effective_hint = _tui_embedded_pane_clarifier(_effective_hint) if _effective_hint: stable_parts.append(_effective_hint) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index 09ccc8459eb1..681002311fdf 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -1084,7 +1084,7 @@ describe('usePromptActions sleep/wake session recovery', () => { expect(ok).toBe(true) // First submit (stale id) → session.resume (stored id) → retry submit (fresh id). expect(calls.map(c => c.method)).toEqual(['prompt.submit', 'session.resume', 'prompt.submit']) - expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID }) + expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID, source: 'desktop' }) expect(calls[2]?.params).toEqual({ session_id: RECOVERED_SESSION_ID, text: 'message after wake' }) }) @@ -1127,7 +1127,7 @@ describe('usePromptActions sleep/wake session recovery', () => { expect(calls.map(c => c.method)).toEqual(['session.interrupt', 'session.resume', 'session.interrupt']) expect(calls[0]?.params).toEqual({ session_id: RUNTIME_SESSION_ID }) - expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID }) + expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID, source: 'desktop' }) expect(calls[2]?.params).toEqual({ session_id: RECOVERED_SESSION_ID }) }) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 5928bae6fd87..8ebe8889d16c 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -546,7 +546,8 @@ export function usePromptActions({ if (isSessionNotFoundError(err) && selectedStoredSessionIdRef.current) { try { const resumed = await requestGateway<{ session_id: string }>('session.resume', { - session_id: selectedStoredSessionIdRef.current + session_id: selectedStoredSessionIdRef.current, + source: 'desktop' }) const recoveredId = resumed?.session_id diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index 5127b534f1c2..09df1221af84 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -260,7 +260,8 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { if (isSessionNotFoundError(firstErr) && selectedStoredSessionIdRef.current) { // Re-register the session in the gateway and get a fresh live ID. const resumed = await requestGateway<{ session_id: string }>('session.resume', { - session_id: selectedStoredSessionIdRef.current + session_id: selectedStoredSessionIdRef.current, + source: 'desktop' }) const recoveredId = resumed?.session_id diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index dc4dcd4176db..e00960de2f2a 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -148,6 +148,12 @@ describe('createBackendSessionForSend profile routing', () => { expect(params).toMatchObject({ profile: 'default' }) }) + it('tags new desktop chats as desktop sessions', async () => { + const params = await createWith(() => {}) + + expect(params).toMatchObject({ source: 'desktop' }) + }) + it('passes the current workspace cwd into session.create', async () => { const params = await createWith(() => { $currentCwd.set('/remote/worktree') @@ -328,6 +334,7 @@ describe('resumeSession failure recovery', () => { expect(resumeParams).not.toHaveProperty('lazy') expect(resumeParams).not.toHaveProperty('eager_build') + expect(resumeParams).toMatchObject({ source: 'desktop' }) }) it('arms the failure latch when resume succeeds with an empty transcript for a non-empty stored session', async () => { @@ -412,6 +419,78 @@ describe('resumeSession failure recovery', () => { }) }) +function BranchHarness({ + onReady, + requestGateway +}: { + onReady: (branchStoredSession: (storedSessionId: string, sessionProfile?: string | null) => Promise) => void + requestGateway: (method: string, params?: Record) => Promise +}) { + const ref = (value: T): MutableRefObject => ({ current: value }) + + const actions = useSessionActions({ + activeSessionId: null, + activeSessionIdRef: ref(null), + busyRef: ref(false), + creatingSessionRef: ref(false), + ensureSessionState: () => ({}) as ClientSessionState, + getRouteToken: () => 'token', + navigate: vi.fn() as never, + requestGateway, + runtimeIdByStoredSessionIdRef: ref(new Map()), + selectedStoredSessionId: null, + selectedStoredSessionIdRef: ref(null), + sessionStateByRuntimeIdRef: ref(new Map()), + syncSessionStateToView: vi.fn(), + updateSessionState: () => ({}) as ClientSessionState + }) + + useEffect(() => { + onReady(actions.branchStoredSession) + }, [actions.branchStoredSession, onReady]) + + return null +} + +describe('branchStoredSession desktop source tagging', () => { + afterEach(() => { + cleanup() + setSessions([]) + vi.restoreAllMocks() + }) + + it('tags desktop branch sessions as desktop sessions', async () => { + let createParams: Record | undefined + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + if (method === 'session.create') { + createParams = params + + return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never + } + + return {} as never + }) + + setSessions([storedSession({ id: 'stored-parent', message_count: 1 })]) + vi.mocked(getSessionMessages).mockResolvedValue({ + messages: [{ content: 'branch me', role: 'user', timestamp: 1 }], + session_id: 'stored-parent' + } as never) + + let branchStoredSession: ((storedSessionId: string) => Promise) | null = null + render( (branchStoredSession = branch)} requestGateway={requestGateway} />) + await waitFor(() => expect(branchStoredSession).not.toBeNull()) + + await expect(branchStoredSession!('stored-parent')).resolves.toBe(true) + + expect(createParams).toMatchObject({ + parent_session_id: 'stored-parent', + source: 'desktop' + }) + }) +}) + // ── Warm-cache mapping integrity (the "open chat A, chat B loads" bug) ───────── // resumeSession's warm fast-path maps storedSessionId -> runtimeId -> cached // state. A reaped/respawned pooled backend re-mints runtime ids, so a recycled diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 0f88fc948100..2b51f002e4b8 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -176,6 +176,7 @@ export function useSessionActions({ const created = await requestGateway('session.create', { cols: 96, + source: 'desktop', ...(cwd && { cwd }), ...(newChatProfile ? { profile: newChatProfile } : {}), ...(uiModel ? { model: uiModel, ...(uiProvider ? { provider: uiProvider } : {}) } : {}), @@ -460,6 +461,7 @@ export function useSessionActions({ const resumePromise = requestGateway('session.resume', { session_id: storedSessionId, cols: 96, + source: 'desktop', // Watch windows attach lazily (live mirror). Every other cold resume // gets the gateway's default deferred build: the RPC returns the // transcript immediately instead of blocking the switch on _make_agent @@ -642,6 +644,7 @@ export function useSessionActions({ // No title: the backend auto-names the branch from its parent's lineage. const branched = await requestGateway('session.create', { cols: 96, + source: 'desktop', ...(cwd && { cwd }), messages: branchMessages.map(({ content, role }) => ({ content, role })), ...(parentStoredId && { parent_session_id: parentStoredId }) diff --git a/tests/agent/test_platform_hint_desktop.py b/tests/agent/test_platform_hint_desktop.py new file mode 100644 index 000000000000..92f90c3affaa --- /dev/null +++ b/tests/agent/test_platform_hint_desktop.py @@ -0,0 +1,246 @@ +"""System-prompt assembly for the desktop chat surface. + +Pins the second half of the fix: the new ``PLATFORM_HINTS["desktop"]`` +entry, the deletion of the standalone desktop-hint block from +``build_environment_hints()``, and the lookup-site extension that +appends the embedded-terminal-pane clarifier to the ``tui`` platform hint +when ``HERMES_DESKTOP_TERMINAL=1``. + +These tests run against the real prompt builders (no mocks) because +cache-stability and byte-for-byte text contracts are what we are +verifying — mocking the resolver would hide exactly the class of bug +this test covers. +""" + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from agent.prompt_builder import PLATFORM_HINTS, build_environment_hints +from agent.system_prompt import ( + _tui_embedded_pane_clarifier, + build_system_prompt_parts, +) + + +def _stable_prompt(agent): + with ( + patch("run_agent.load_soul_md", return_value=""), + patch("run_agent.build_nous_subscription_prompt", return_value=""), + patch("run_agent.build_environment_hints", return_value=""), + patch("run_agent.build_context_files_prompt", return_value=""), + ): + return build_system_prompt_parts(agent)["stable"] + + +def _make_agent(platform="", **overrides): + base = dict( + load_soul_identity=False, + skip_context_files=False, + valid_tool_names=[], + _task_completion_guidance=False, + _tool_use_enforcement=False, + _environment_probe=False, + _kanban_worker_guidance="", + _memory_store=None, + _memory_manager=None, + _platform_hint_overrides={}, + model="", + provider="", + pass_session_id=False, + session_id="", + ) + base["platform"] = platform + base.update(overrides) + return SimpleNamespace(**base) + + +class TestDesktopHintEntry: + def test_desktop_key_exists(self): + """The map must carry a "desktop" entry — without it the platform + hint lookup falls through to an empty string and the agent gets no + surface framing at all on the desktop chat surface.""" + assert "desktop" in PLATFORM_HINTS + + def test_desktop_hint_disambiguates_from_terminal(self): + """The agent must be told it is in a graphical chat surface, NOT a + terminal. This is the line that kills the contradiction with the + old tui mis-tag.""" + hint = PLATFORM_HINTS["desktop"] + lowered = hint.lower() + assert "desktop" in lowered + assert "not a terminal" in lowered + assert "graphical chat surface" in lowered + + def test_desktop_hint_advertises_markdown(self): + """The desktop renderer supports full GFM (verified via the + Streamdown pipeline in apps/desktop). The hint must steer the + agent toward markdown, not away from it like the cli/tui hints do.""" + hint = PLATFORM_HINTS["desktop"] + assert "markdown" in hint.lower() + + def test_desktop_hint_advertises_media_delivery(self): + """The desktop chat intercepts MEDIA:/abs/path like telegram — images + inline, audio/video inline players, other files as download links. + Without this line the agent falls back to the cli/tui "state the + path in text" model, which is the wrong UX for the desktop surface.""" + hint = PLATFORM_HINTS["desktop"] + assert "MEDIA:" in hint + + def test_desktop_hint_advertises_inline_image_urls(self): + hint = PLATFORM_HINTS["desktop"] + assert "![alt](url)" in hint + + def test_desktop_hint_does_not_inherit_tui_cron_local_only_block(self): + """The desktop chat surface's cron delivery semantics differ from + the standalone TUI — desktop runs its own cron ticker in-process + (hermes_cli/web_server.py under HERMES_DESKTOP=1). We deliberately + do NOT parrot the tui "LOCAL-ONLY … no live-delivery channel" block + into the desktop hint, since partially-correct cron guidance is + exactly the bug class we are fixing. Cron guidance for desktop is + deferred to a follow-up issue.""" + hint = PLATFORM_HINTS["desktop"] + assert "LOCAL-ONLY" not in hint + + +class TestDesktopHintBlockRemoved: + """The standalone desktop-hint block that used to live in + ``build_environment_hints()`` (lines ~1130-1144) was a band-aid for the + missing ``PLATFORM_HINTS["desktop"]`` entry. Once that entry exists, the + block is dead code that competes with the platform hint's claim of + what surface the agent is on. It must be gone.""" + + def test_build_environment_hints_has_no_runtime_surface_line(self, monkeypatch): + monkeypatch.setenv("HERMES_DESKTOP", "1") + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + from agent.prompt_builder import _clear_backend_probe_cache + _clear_backend_probe_cache() + hints = build_environment_hints() + assert "Runtime surface:" not in hints + assert "desktop GUI app" not in hints + + def test_build_environment_hints_has_no_embedded_pane_clarifier(self, monkeypatch): + """The ⌥-drag / ⌘+L embedded-pane clarifier moves to the platform-hint + resolution site (system_prompt.py), not build_environment_hints().""" + monkeypatch.setenv("HERMES_DESKTOP", "1") + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", "1") + from agent.prompt_builder import _clear_backend_probe_cache + _clear_backend_probe_cache() + hints = build_environment_hints() + assert "embedded terminal pane" not in hints + assert "Shift-drag" not in hints + + +class TestPlatformHintResolutionInStablePrompt: + """End-to-end through ``build_system_prompt_parts`` — the platform tag on + the agent drives BOTH which PLATFORM_HINTS entry gets appended AND + whether the embedded-pane clarifier follows it. The desktop-hint block + that used to live in ``build_environment_hints()`` is gone.""" + + def test_desktop_platform_yields_desktop_hint_no_tui_framing(self, monkeypatch): + monkeypatch.setenv("HERMES_DESKTOP", "1") + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + stable = _stable_prompt(_make_agent(platform="desktop")) + assert PLATFORM_HINTS["desktop"] in stable + assert "terminal UI" not in stable + assert "Runtime surface:" not in stable + assert "embedded terminal pane" not in stable + + def test_standalone_tui_yields_plain_tui_hint_no_clarifier(self, monkeypatch): + monkeypatch.delenv("HERMES_DESKTOP", raising=False) + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + stable = _stable_prompt(_make_agent(platform="tui")) + assert PLATFORM_HINTS["tui"] in stable + assert "embedded terminal pane" not in stable + + def test_embedded_tui_yields_tui_hint_with_clarifier(self, monkeypatch): + monkeypatch.setenv("HERMES_DESKTOP", "1") + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", "1") + stable = _stable_prompt(_make_agent(platform="tui")) + assert PLATFORM_HINTS["tui"] in stable + assert "embedded terminal pane" in stable + assert "Shift-drag" in stable or "Option-drag" in stable or "⌥" in stable + + def test_embedded_clarifier_does_not_attach_to_desktop_platform(self, monkeypatch): + """Critical regression: even when HERMES_DESKTOP_TERMINAL=1, a + desktop-tagged session must NOT get the embedded-pane clarifier — + the clarifier describes the *embedded terminal pane*, which a + desktop chat session is not.""" + monkeypatch.setenv("HERMES_DESKTOP", "1") + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", "1") + stable = _stable_prompt(_make_agent(platform="desktop")) + assert "embedded terminal pane" not in stable + + +class TestEmbeddedTuiPaneClarifier: + """When ``HERMES_DESKTOP_TERMINAL=1``, a standalone ``hermes --tui`` is + running inside the desktop's embedded terminal pane. The user can + ⌥-drag-select its output and ⌘/Ctrl+L to send it to the chat composer. + That clarifier must be appended to the ``tui`` platform hint at the + resolution site, NOT baked into the static ``PLATFORM_HINTS["tui"]`` + string (which is shared with every standalone TUI session and must + stay byte-stable).""" + + def test_tui_standalone_hint_byte_stable_without_env(self, monkeypatch): + """Without HERMES_DESKTOP_TERMINAL, the clarifier is a no-op and the + resolved tui hint is exactly the static PLATFORM_HINTS["tui"] + string. Cache-stable for every standalone TUI session.""" + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + out = _tui_embedded_pane_clarifier(PLATFORM_HINTS["tui"]) + assert out == PLATFORM_HINTS["tui"] + + def test_embedded_pane_clarifier_appended_when_env_set(self, monkeypatch): + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", "1") + out = _tui_embedded_pane_clarifier(PLATFORM_HINTS["tui"]) + assert out.startswith(PLATFORM_HINTS["tui"]) + assert "embedded terminal pane" in out + assert "Shift-drag" in out or "Option-drag" in out or "⌥" in out + + def test_embedded_pane_clarifier_idempotent(self, monkeypatch): + """Calling the clarifier twice must NOT double-append the sentence. + Cache-stability: the resolver is called once per session build, so + re-applying on an already-augmented hint is a no-op.""" + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", "1") + once = _tui_embedded_pane_clarifier(PLATFORM_HINTS["tui"]) + twice = _tui_embedded_pane_clarifier(once) + assert once == twice + + def test_embedded_pane_clarifier_does_not_touch_empty_hint(self, monkeypatch): + """Defensive: if the tui hint is somehow empty (e.g. overridden to + empty by config), do not synthesize a clarifier-only hint — that + would put a desktop-pane reference in the prompt without the tui + surface framing it sits under.""" + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", "1") + out = _tui_embedded_pane_clarifier("") + assert out == "" + + @pytest.mark.parametrize("val", ["0", "false", "no", "", "0", "False"]) + def test_falsy_env_does_not_trigger_clarifier(self, monkeypatch, val): + monkeypatch.setenv("HERMES_DESKTOP_TERMINAL", val) + out = _tui_embedded_pane_clarifier(PLATFORM_HINTS["tui"]) + assert out == PLATFORM_HINTS["tui"], ( + f"HERMES_DESKTOP_TERMINAL={val!r} should not trigger clarifier" + ) + + +class TestContradictionGone: + """The original contradiction: a single assembled system prompt + contained both ``You are running in the Hermes terminal UI (TUI).`` and + ``Runtime surface: you're running inside the Hermes desktop GUI app.``. + After the fix, no single session's prompt can carry both.""" + + def test_desktop_chat_session_has_no_tui_framing(self, monkeypatch): + monkeypatch.setenv("HERMES_DESKTOP", "1") + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + assert "tui" in PLATFORM_HINTS + assert "desktop" in PLATFORM_HINTS + desktop_hint = PLATFORM_HINTS["desktop"] + tui_hint = PLATFORM_HINTS["tui"] + assert "terminal UI" not in desktop_hint + assert "terminal UI" in tui_hint + + def test_tui_hint_does_not_carry_desktop_marker(self): + tui_hint = PLATFORM_HINTS["tui"] + assert "desktop GUI app" not in tui_hint + assert "Runtime surface:" not in tui_hint diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 6f58d35d82c9..50a9fa8d5ce6 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -13,6 +13,7 @@ import pytest from hermes_constants import reset_hermes_home_override, set_hermes_home_override from hermes_cli.active_sessions import active_session_registry_snapshot +from hermes_cli.browser_connect import ChromeDebugLaunch from tui_gateway import server @@ -1827,7 +1828,7 @@ def test_session_close_commits_memory_and_fires_finalize_hook(monkeypatch): monkeypatch.setattr( server, "_notify_session_boundary", - lambda event, session_id: calls["hooks"].append((event, session_id)), + lambda event, session_id, *_args: calls["hooks"].append((event, session_id)), ) try: @@ -1959,7 +1960,7 @@ def test_init_session_fires_reset_hook(monkeypatch): monkeypatch.setattr( server, "_notify_session_boundary", - lambda event, session_id: hooks.append((event, session_id)), + lambda event, session_id, *_args: hooks.append((event, session_id)), ) import tools.approval as _approval @@ -5498,7 +5499,7 @@ def test_session_create_close_race_does_not_orphan_worker(monkeypatch): release_build = threading.Event() build_entered = threading.Event() - def _slow_make_agent(sid, key, session_id=None, session_db=None): + def _slow_make_agent(sid, key, session_id=None, session_db=None, **_kwargs): build_started.set() build_entered.set() release_build.wait(timeout=3.0) @@ -5606,7 +5607,7 @@ def test_session_create_no_race_keeps_worker_alive(monkeypatch): self.base_url = "" self.api_key = "" - monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_db=None: _FakeAgent()) + monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_db=None, **_kwargs: _FakeAgent()) monkeypatch.setattr(server, "_SlashWorker", _FakeWorker) monkeypatch.setattr( server, @@ -5712,7 +5713,7 @@ def test_session_create_continues_when_state_db_is_unavailable(monkeypatch): emits = [] - monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_db=None: _FakeAgent()) + monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_db=None, **_kwargs: _FakeAgent()) monkeypatch.setattr(server, "_SlashWorker", _FakeWorker) monkeypatch.setattr(server, "_get_db", lambda: None) monkeypatch.setattr(server, "_session_info", lambda _a, *a2: {"model": "x"}) @@ -6810,8 +6811,10 @@ def test_browser_manage_connect_default_local_reports_launch_hint(monkeypatch): _stub_urlopen(monkeypatch, ok=False) with ( patch( - "hermes_cli.browser_connect.try_launch_chrome_debug", return_value=False + "hermes_cli.browser_connect.launch_chrome_debug", + return_value=ChromeDebugLaunch(), ), + patch("hermes_cli.browser_connect.manual_chrome_debug_command", return_value=None), patch( "hermes_cli.browser_connect.get_chrome_debug_candidates", return_value=[], @@ -6866,8 +6869,10 @@ def test_browser_manage_connect_no_session_skips_progress_events(monkeypatch): _stub_urlopen(monkeypatch, ok=False) with ( patch( - "hermes_cli.browser_connect.try_launch_chrome_debug", return_value=False + "hermes_cli.browser_connect.launch_chrome_debug", + return_value=ChromeDebugLaunch(), ), + patch("hermes_cli.browser_connect.manual_chrome_debug_command", return_value=None), patch( "hermes_cli.browser_connect.get_chrome_debug_candidates", return_value=[], diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 170e78aec643..5aa3daa00ec5 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -315,7 +315,7 @@ def test_session_resume_returns_hydrated_messages(server, monkeypatch): ] monkeypatch.setattr(server, "_get_db", lambda: _DB()) - monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_id=None, session_db=None: object()) + monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_id=None, session_db=None, **_kwargs: object()) monkeypatch.setattr(server, "_init_session", lambda sid, key, agent, history, cols=80, **_kwargs: None) monkeypatch.setattr(server, "_session_info", lambda _agent, _session=None: {"model": "test/model"}) @@ -509,7 +509,7 @@ def test_session_resume_handles_multimodal_list_content(server, monkeypatch): return [multimodal_user, text_only_assistant] monkeypatch.setattr(server, "_get_db", lambda: _DB()) - monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_id=None, session_db=None: object()) + monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_id=None, session_db=None, **_kwargs: object()) monkeypatch.setattr(server, "_init_session", lambda sid, key, agent, history, cols=80, **_kwargs: None) monkeypatch.setattr(server, "_session_info", lambda _agent, _session=None: {"model": "test/model"}) @@ -795,7 +795,7 @@ def test_session_resume_reuses_existing_live_session(server, monkeypatch): def close(self): closed_sids.append(self.sid) - def make_agent(sid, key, session_id=None, session_db=None): + def make_agent(sid, key, session_id=None, session_db=None, **_kwargs): created_sids.append(sid) first_agent_started.set() assert agent_can_finish.wait(timeout=1) @@ -1006,7 +1006,7 @@ def test_session_resume_live_payload_uses_current_history_with_ancestors(server, monkeypatch.setattr( server, "_make_agent", - lambda _sid, key, session_id=None, session_db=None: types.SimpleNamespace( + lambda _sid, key, session_id=None, session_db=None, **_kwargs: types.SimpleNamespace( model="test/model", session_id=session_id or key ), ) @@ -1144,7 +1144,7 @@ def test_session_branch_persists_branched_from_marker(server, monkeypatch): monkeypatch.setattr( server, "_make_agent", - lambda _sid, key, session_id=None, session_db=None: types.SimpleNamespace( + lambda _sid, key, session_id=None, session_db=None, **_kwargs: types.SimpleNamespace( model="test/model", session_id=session_id or key ), ) diff --git a/tests/tui_gateway/test_session_platform_resolution.py b/tests/tui_gateway/test_session_platform_resolution.py new file mode 100644 index 000000000000..da241530604e --- /dev/null +++ b/tests/tui_gateway/test_session_platform_resolution.py @@ -0,0 +1,147 @@ +"""Platform/source tagging for the desktop chat surface. + +The desktop app's chat panel uses ``hermes serve`` (the ``tui_gateway`` +backend), so every chat session historically got ``platform="tui"`` stamped +on it — even though the user is in a graphical chat surface, not a +terminal. That mis-tag is why the agent suggested TUI-only slash commands +(like ``/reload-mcp``) to desktop chat users. + +These tests pin the env-var matrix that resolves the session platform at +``tui_gateway`` session-creation time: + + HERMES_DESKTOP=1, HERMES_DESKTOP_TERMINAL unset -> platform="desktop" + HERMES_DESKTOP=1, HERMES_DESKTOP_TERMINAL=1 -> platform="tui" (embedded pane) + neither set -> platform="tui" (standalone) + +The resolver helper is import-safe (no heavy module side effects) so it +can be unit-tested without spinning up the full gateway. +""" + +import importlib + +import pytest + + +def _reload_resolver(): + import tui_gateway.server as _srv + importlib.reload(_srv) + return _srv + + +@pytest.fixture +def clean_env(monkeypatch): + monkeypatch.delenv("HERMES_DESKTOP", raising=False) + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + return monkeypatch + + +class TestResolveSessionPlatform: + def test_standalone_tui_neither_env_set(self, clean_env): + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "tui" + + def test_desktop_chat_backend_gets_desktop_tag(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "desktop" + + def test_desktop_embedded_terminal_pane_stays_tui(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + clean_env.setenv("HERMES_DESKTOP_TERMINAL", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "tui" + + def test_desktop_terminal_alone_means_standalone_tui(self, clean_env): + clean_env.setenv("HERMES_DESKTOP_TERMINAL", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "tui" + + @pytest.mark.parametrize("val", ["1", "true", "yes", "on", "TRUE", "Yes", "ON"]) + def test_truthy_variants_recognized(self, clean_env, val): + clean_env.setenv("HERMES_DESKTOP", val) + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "desktop" + + @pytest.mark.parametrize("val", ["0", "false", "", "no", "off", "False"]) + def test_falsy_variants_fall_back_to_tui(self, clean_env, val): + clean_env.setenv("HERMES_DESKTOP", val) + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "tui" + + def test_embedded_terminal_overrides_desktop_when_both_set(self, clean_env): + """The terminal-pane qualifier must short-circuit the desktop-backend + marker. An embedded TUI is a TUI, not a desktop chat surface.""" + clean_env.setenv("HERMES_DESKTOP", "1") + clean_env.setenv("HERMES_DESKTOP_TERMINAL", "true") + _srv = _reload_resolver() + assert _srv._resolve_session_platform() == "tui" + + +class TestResolveSessionSource: + def test_explicit_source_param_wins(self, clean_env): + _srv = _reload_resolver() + assert _srv._resolve_session_source("telegram") == "telegram" + + def test_explicit_empty_source_falls_back_to_env(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_source("") == "desktop" + + def test_explicit_none_source_falls_back_to_env(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_source(None) == "desktop" + + def test_no_env_no_param_defaults_to_tui(self, clean_env): + _srv = _reload_resolver() + assert _srv._resolve_session_source(None) == "tui" + + def test_embedded_terminal_default_is_tui(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + clean_env.setenv("HERMES_DESKTOP_TERMINAL", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_source(None) == "tui" + + def test_explicit_source_param_resists_env_drift(self, clean_env): + """A caller that explicitly passes source="cli" must not be silently + rewritten to "desktop" by env vars — the resolver only fills in the + default when one is missing.""" + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._resolve_session_source("cli") == "cli" + + +class TestResolveAgentPlatform: + def test_explicit_desktop_source_drives_agent_platform_without_env(self, clean_env): + _srv = _reload_resolver() + assert _srv._resolve_agent_platform("desktop") == "desktop" + + def test_missing_source_falls_back_to_env_resolved_platform(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._resolve_agent_platform(None) == "desktop" + + def test_explicit_tui_source_keeps_embedded_terminal_as_tui(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._resolve_agent_platform("tui") == "tui" + + +class TestSessionSourceFallback: + def test_session_source_uses_existing_session_value(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._session_source({"source": "telegram"}) == "telegram" + + def test_session_source_defaults_to_desktop_under_desktop_backend(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + _srv = _reload_resolver() + assert _srv._session_source({}) == "desktop" + assert _srv._session_source(None) == "desktop" + + def test_session_source_defaults_to_tui_for_embedded_terminal(self, clean_env): + clean_env.setenv("HERMES_DESKTOP", "1") + clean_env.setenv("HERMES_DESKTOP_TERMINAL", "1") + _srv = _reload_resolver() + assert _srv._session_source({}) == "tui" + assert _srv._session_source(None) == "tui" diff --git a/tui_gateway/server.py b/tui_gateway/server.py index fb98f615fa30..8092147bc79c 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -404,12 +404,18 @@ def _load_busy_input_mode() -> str: return raw if raw in {"queue", "steer", "interrupt"} else "interrupt" -def _notify_session_boundary(event_type: str, session_id: str | None) -> None: +def _notify_session_boundary( + event_type: str, session_id: str | None, platform: str | None = None +) -> None: """Fire session lifecycle hooks with CLI parity.""" try: from hermes_cli.plugins import invoke_hook as _invoke_hook - _invoke_hook(event_type, session_id=session_id, platform="tui") + _invoke_hook( + event_type, + session_id=session_id, + platform=_resolve_agent_platform(platform), + ) except Exception: pass @@ -477,6 +483,7 @@ def _transfer_active_session_slot( new_lease, limit_message = _claim_active_session_slot( new_session_id, live_session_id=sid, + surface=_session_source(session), ) if new_lease is not None: old_lease = session.pop("active_session_lease", None) @@ -612,7 +619,7 @@ def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> No session_key = session.get("session_key") session_id = getattr(agent, "session_id", None) or session_key - _notify_session_boundary("on_session_finalize", session_id) + _notify_session_boundary("on_session_finalize", session_id, _session_source(session)) # Mark session ended in DB so it doesn't linger as a ghost row in /resume. # Use session_id (from agent.session_id) not session_key — after compression, @@ -1322,6 +1329,7 @@ def _start_agent_build(sid: str, session: dict) -> None: kw = {"session_db": session_db} if resume_sid := current.get("resume_session_id"): kw["session_id"] = resume_sid + kw["platform_override"] = _session_source(current) resume_overrides = current.get("resume_runtime_overrides") if isinstance(resume_overrides, dict) and resume_overrides: # Cold deferred resume: restore the full persisted runtime @@ -1401,7 +1409,7 @@ def _start_agent_build(sid: str, session: dict) -> None: with _sessions_lock: if sid in _sessions: _sessions[sid]["_notif_stop"] = _start_notification_poller(sid, _sessions[sid]) - _notify_session_boundary("on_session_reset", key) + _notify_session_boundary("on_session_reset", key, _session_source(current)) info = _session_info(agent, current) cfg_warn = _probe_config_health(_load_cfg()) @@ -1603,7 +1611,7 @@ def _session_source(session: dict | None) -> str: source = str(session.get("source") or "").strip() if source: return source - return "tui" + return _resolve_session_platform() def _register_session_cwd(session: dict | None) -> None: @@ -1947,7 +1955,7 @@ def _set_session_context(session_key: str, cwd: str | None = None) -> list: # know the parent workspace pass it explicitly so spawned agents inherit # it instead of falling back to the gateway launch dir. resolved = cwd if cwd is not None else _cwd_for_session_key(session_key) - source = "tui" + source = _resolve_session_platform() with _sessions_lock: for sess in list(_sessions.values()): if sess.get("session_key") == session_key: @@ -2050,6 +2058,49 @@ def _resolve_model() -> str: return "anthropic/claude-sonnet-4" +def _resolve_session_platform() -> str: + """Resolve the platform tag for a tui_gateway-routed session. + + The desktop app's chat panel and the standalone TUI both speak to this + gateway; without a branch they all get stamped ``platform="tui"``, + which makes the agent think it's talking to a terminal user. That + mis-tag is the root cause of the desktop chat agent suggesting + TUI-only slash commands (``/reload-mcp``, …) to chat-panel users. + + Resolution: + * ``HERMES_DESKTOP=1`` and ``HERMES_DESKTOP_TERMINAL`` unset → "desktop" + (the chat-panel backend — a graphical React surface, not a terminal). + * ``HERMES_DESKTOP_TERMINAL=1`` → "tui" + (``hermes --tui`` running in the desktop's embedded terminal pane; + it IS a TUI, just embedded. The clarifier attached to the tui hint + in system_prompt.py tells the agent about the embedding.) + * neither set → "tui" + (standalone ``hermes --tui``.) + """ + if is_truthy_value(os.environ.get("HERMES_DESKTOP")) and not is_truthy_value( + os.environ.get("HERMES_DESKTOP_TERMINAL") + ): + return "desktop" + return "tui" + + +def _resolve_session_source(explicit: str | None) -> str: + """Default the session DB ``source`` field from the resolved platform. + + A caller that explicitly passes ``source`` (e.g. a plugin session tagged + ``"telegram"``) keeps its value. Only an empty/None ``source`` falls back + to the env-resolved platform — so env-driven resolution never silently + rewrites a caller's intent. + """ + if explicit: + return explicit + return _resolve_session_platform() + + +def _resolve_agent_platform(source: str | None) -> str: + return _resolve_session_source(source) + + def _config_model_target() -> tuple[str, str]: """(model, provider) currently selected by config.yaml — and ONLY config. @@ -2528,7 +2579,7 @@ def _load_enabled_toolsets() -> list[str] | None: try: from agent.coding_context import coding_selection - selection = coding_selection(platform="tui") + selection = coding_selection(platform=_resolve_session_platform()) if selection is not None: # Fold in `project` here too: this is a GUI-only resolver, and # the focus-mode coding posture returns before the fallback path @@ -4222,6 +4273,7 @@ def _reset_session_agent(sid: str, session: dict) -> dict: sid, session["session_key"], session_id=session["session_key"], + platform_override=_session_source(session), **reset_kw, ) finally: @@ -4371,6 +4423,7 @@ def _make_agent( provider_override: str | None = None, reasoning_config_override: dict | None = None, service_tier_override: str | None = None, + platform_override: str | None = None, ): from run_agent import AIAgent @@ -4513,7 +4566,7 @@ def _make_agent( provider_sort=_pr.get("sort"), provider_require_parameters=_pr.get("require_parameters", False), provider_data_collection=_pr.get("data_collection"), - platform="tui", + platform=_resolve_agent_platform(platform_override), session_id=session_id or key, session_db=session_db if session_db is not None else _get_db(), ephemeral_system_prompt=system_prompt or None, @@ -4534,6 +4587,7 @@ def _init_session( cols: int = 80, cwd: str | None = None, session_db=None, + source: str | None = None, ): now = time.time() with _sessions_lock: @@ -4553,6 +4607,7 @@ def _init_session( "cols": cols, "slash_worker": None, "show_reasoning": _load_show_reasoning(), + "source": _resolve_session_source(source), "tool_progress_mode": _load_tool_progress_mode(), "edit_snapshots": {}, "tool_started_at": {}, @@ -4621,7 +4676,7 @@ def _init_session( with _sessions_lock: if sid in _sessions: _sessions[sid]["_notif_stop"] = _start_notification_poller(sid, _sessions[sid]) - _notify_session_boundary("on_session_reset", key) + _notify_session_boundary("on_session_reset", key, _session_source(_sessions.get(sid, {}))) _emit("session.info", sid, _session_info(agent, _sessions.get(sid, {}))) _schedule_mcp_late_refresh(sid, agent) @@ -5067,7 +5122,7 @@ def _(rid, params: dict) -> dict: except Exception: explicit_cwd = False resolved_cwd = _completion_cwd(params) - source = str(params.get("source") or "tui").strip() or "tui" + source = _resolve_session_source(str(params.get("source") or "").strip() or None) _enable_gateway_prompts() # ``profile`` (app-global remote mode): a new chat started under a non-launch @@ -5102,7 +5157,9 @@ def _(rid, params: dict) -> dict: ready = threading.Event() now = time.time() - lease, limit_message = _claim_active_session_slot(key, live_session_id=sid) + lease, limit_message = _claim_active_session_slot( + key, live_session_id=sid, surface=source + ) if limit_message is not None: return _err(rid, 4090, limit_message) @@ -5528,7 +5585,10 @@ def _(rid, params: dict) -> dict: # (resume_session_id keeps the upgrade on the stored conversation). if is_truthy_value(params.get("lazy", False)): sid = uuid.uuid4().hex[:8] - lease, limit_message = _claim_active_session_slot(target, live_session_id=sid) + source = _resolve_session_source(str(params.get("source") or "").strip() or None) + lease, limit_message = _claim_active_session_slot( + target, live_session_id=sid, surface=source + ) if limit_message is not None: return _err(rid, 4090, limit_message) try: @@ -5547,7 +5607,7 @@ def _(rid, params: dict) -> dict: cwd=cwd, history=history, lease=lease, - source=str(params.get("source") or "tui").strip() or "tui", + source=source, close_on_disconnect=is_truthy_value(params.get("close_on_disconnect", False)), profile_home=profile_home, lazy=True, @@ -5589,7 +5649,10 @@ def _(rid, params: dict) -> dict: # session's persisted runtime identity, and is a real (upgradable) session. if not is_truthy_value(params.get("eager_build", False)): sid = uuid.uuid4().hex[:8] - lease, limit_message = _claim_active_session_slot(target, live_session_id=sid) + source = _resolve_session_source(str(params.get("source") or "").strip() or None) + lease, limit_message = _claim_active_session_slot( + target, live_session_id=sid, surface=source + ) if limit_message is not None: return _err(rid, 4090, limit_message) # Interactive resume routes approvals/clarify through gateway prompts; @@ -5620,7 +5683,7 @@ def _(rid, params: dict) -> dict: cwd=cwd, history=history, lease=lease, - source=str(params.get("source") or "tui").strip() or "tui", + source=source, close_on_disconnect=is_truthy_value(params.get("close_on_disconnect", False)), display_history_prefix=prefix, profile_home=profile_home, @@ -5659,7 +5722,10 @@ def _(rid, params: dict) -> dict: # _session_resume_lock across it would stall session.close on the main # dispatch thread (it's not a _LONG_HANDLER), blocking fast-path RPCs. sid = uuid.uuid4().hex[:8] - lease, limit_message = _claim_active_session_slot(target, live_session_id=sid) + source = _resolve_session_source(str(params.get("source") or "").strip() or None) + lease, limit_message = _claim_active_session_slot( + target, live_session_id=sid, surface=source + ) if limit_message is not None: return _err(rid, 4090, limit_message) _enable_gateway_prompts() @@ -5697,6 +5763,7 @@ def _(rid, params: dict) -> dict: target, session_id=target, session_db=db, + platform_override=source, **stored_runtime_overrides, ) finally: @@ -5747,6 +5814,7 @@ def _(rid, params: dict) -> dict: cols=cols, cwd=profile_resume_cwd, session_db=db, + source=source, ) finally: if init_home_token is not None: @@ -7917,7 +7985,10 @@ def _(rid, params: dict) -> dict: return _err(rid, 4008, "nothing to branch — send a message first") new_key = _new_session_key() new_sid = uuid.uuid4().hex[:8] - lease, limit_message = _claim_active_session_slot(new_key, live_session_id=new_sid) + source = _session_source(session) + lease, limit_message = _claim_active_session_slot( + new_key, live_session_id=new_sid, surface=source + ) if limit_message is not None: return _err(rid, 4090, limit_message) branch_name = params.get("name", "") @@ -7933,7 +8004,7 @@ def _(rid, params: dict) -> dict: ) db.create_session( new_key, - source=_session_source(session), + source=source, model=_resolve_model(), # Stable _branched_from marker so list_sessions_rich() keeps the # branch visible in /resume and /sessions. The TUI branch leaves @@ -7958,11 +8029,21 @@ def _(rid, params: dict) -> dict: try: tokens = _set_session_context(new_key) try: - agent = _make_agent(new_sid, new_key, session_id=new_key) + agent = _make_agent( + new_sid, + new_key, + session_id=new_key, + platform_override=source, + ) finally: _clear_session_context(tokens) _init_session( - new_sid, new_key, agent, list(history), cols=session.get("cols", 80) + new_sid, + new_key, + agent, + list(history), + cols=session.get("cols", 80), + source=source, ) if new_sid in _sessions: _sessions[new_sid]["active_session_lease"] = lease