fix(mcp): surface backend errors in Agent UI MCP callers + stop session nav oscillation

src/gaia/apps/webui/src/App.tsx

@@ -291,33 +291,39 @@ function App() {
         };
     }, [setRunningSessions]);
 
-    // Support URL-based session navigation (?session=<id> or #<hash>)
+    // Support URL-based session navigation (?session=<id> or #<hash>).
+    // Runs once on mount only. Depending on currentSessionId caused a
+    // ping-pong (#1755): the hash-sync effect writes a 7-char short hash that
+    // never equals the full UUID, so on every session switch this guard re-fired,
+    // read a stale hash, and scheduled a ~500ms bounce back to the URL's target.
+    // currentSessionId is read fresh inside the timer instead of via deps.
     useEffect(() => {
         const params = new URLSearchParams(window.location.search);
         const sessionParam = params.get('session');
         const hashParam = window.location.hash.replace(/^#/, '');
         const target = sessionParam || hashParam;
         if (!target) return;
-        // Already on this session — no switch needed
-        if (target === currentSessionId) return;
 
         log.nav.info(`URL session parameter: ${target}`);
         // Defer so session list has time to load
         const timer = setTimeout(() => {
-            const { sessions } = useChatStore.getState();
+            const { sessions, currentSessionId } = useChatStore.getState();
+            // Already on this session — no switch needed
+            if (target === currentSessionId) return;
             // Try exact match first (full UUID), then short hash match
-            let matchId: string | null = sessions.some((s: { id: string }) => s.id === target)
+            const matchId: string | null = sessions.some((s: { id: string }) => s.id === target)
                 ? target
                 : findSessionByHash(sessions, target);
-            if (matchId) {
+            if (matchId && matchId !== currentSessionId) {
                 setCurrentSession(matchId);
                 setMessages([]);
-            } else {
+            } else if (!matchId) {
                 log.nav.warn(`Session ${target} not found in loaded sessions`);
             }
         }, 500);
         return () => clearTimeout(timer);
-    }, [currentSessionId, setCurrentSession, setMessages]);
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [setCurrentSession, setMessages]);
 
     // Subscribe to system session-activation events (MCP bridge P1)
     useEffect(() => {

src/gaia/apps/webui/src/__tests__/App.test.tsx

@@ -2,35 +2,107 @@
 // SPDX-License-Identifier: MIT
 
 /**
- * App.tsx integration tests — MCP/SSE session activation (issue #1086).
+ * App.tsx nav-guard tests — URL/hash session navigation (issues #1086, #1755).
+ *
+ * Renders the real <App> with every child stubbed so only App's own effects
+ * run, then drives the URL-hash navigation guard with fake timers.
  */
 
-import { describe, it, expect } from 'vitest';
-
-describe('App session navigation guard', () => {
-    it('hash URL format includes # not ?session=', () => {
-        // Verify the contract: bridge uses /#<id>, not /?session=<id>
-        const sessionId = 'abc123';
-        const hashUrl = `http://localhost:4200/#${sessionId}`;
-        const queryUrl = `http://localhost:4200/?session=${sessionId}`;
-        expect(hashUrl).toContain('#');
-        expect(hashUrl).not.toContain('?session=');
-        expect(queryUrl).not.toContain('#');
-    });
+import { render, act } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import App from '../App';
+import { useChatStore } from '../stores/chatStore';
+import * as api from '../services/api';
+import { getSessionHash } from '../utils/format';
+
+// Stub every child so their mount effects don't interfere with the guard.
+vi.mock('../components/Sidebar', () => ({ Sidebar: () => null }));
+vi.mock('../components/ChatView', () => ({ ChatView: () => null }));
+vi.mock('../components/WelcomeScreen', () => ({ WelcomeScreen: () => null }));
+vi.mock('../components/DocumentLibrary', () => ({ DocumentLibrary: () => null }));
+vi.mock('../components/FileBrowser', () => ({ FileBrowser: () => null }));
+vi.mock('../components/MemoryDashboard', () => ({ MemoryDashboard: () => null }));
+vi.mock('../components/ScheduleManager', () => ({ ScheduleManager: () => null }));
+vi.mock('../components/SettingsPage', () => ({ SettingsPage: () => null }));
+vi.mock('../components/MobileAccessModal', () => ({ MobileAccessModal: () => null }));
+vi.mock('../components/ConnectionBanner', () => ({ ConnectionBanner: () => null }));
+vi.mock('../components/UpdateIndicator', () => ({ UpdateIndicator: () => null }));
+vi.mock('../components/PermissionPrompt', () => ({ PermissionPrompt: () => null }));
+vi.mock('../components/NotificationCenter', () => ({ NotificationCenter: () => null }));
+vi.mock('../services/api');
+
+const SESSION_A = { id: 'aaaaaaaa-1111-2222-3333-444444444444' };
+const SESSION_B = { id: 'bbbbbbbb-5555-6666-7777-888888888888' };
+
+const mockedApi = vi.mocked(api);
 
-    it('set_active_session event shape matches contract', () => {
-        // Verify the SSE event shape the frontend expects
-        const event = { type: 'set_active_session', session_id: 'test-123' };
-        expect(event.type).toBe('set_active_session');
-        expect(event.session_id).toBeDefined();
-        expect(typeof event.session_id).toBe('string');
+// jsdom has no EventSource; provide a no-op so App's SSE effect can mount.
+class FakeEventSource {
+    onopen: (() => void) | null = null;
+    onmessage: ((ev: { data: string }) => void) | null = null;
+    onerror: (() => void) | null = null;
+    close() { /* no-op */ }
+}
+
+beforeEach(() => {
+    vi.clearAllMocks();
+    (globalThis as unknown as { EventSource: unknown }).EventSource = FakeEventSource;
+
+    mockedApi.listAgents.mockResolvedValue({ agents: [] } as never);
+    mockedApi.getSystemStatus.mockResolvedValue({ lemonade_running: true } as never);
+    mockedApi.listSessions.mockResolvedValue({ sessions: [SESSION_A, SESSION_B] } as never);
+    mockedApi.getActiveRuns.mockResolvedValue({ session_ids: [] } as never);
+    mockedApi.getTunnelStatus.mockResolvedValue({ active: false } as never);
+
+    useChatStore.setState({
+        sessions: [SESSION_A, SESSION_B] as never,
+        currentSessionId: null,
+        messages: [],
     });
+    window.history.replaceState(null, '', '/');
+});
+
+afterEach(() => {
+    vi.useRealTimers();
 });
 
-describe('index.html title', () => {
-    it('title is GAIA Agent UI', () => {
-        // This test documents the required title value
-        const expectedTitle = 'GAIA Agent UI';
-        expect(expectedTitle).toBe('GAIA Agent UI');
+describe('App URL/hash session navigation guard', () => {
+    it('navigates to the session named in the URL hash on mount', async () => {
+        vi.useFakeTimers();
+        window.history.replaceState(null, '', `#${getSessionHash(SESSION_A.id)}`);
+
+        await act(async () => {
+            render(<App />);
+        });
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(600);
+        });
+
+        expect(useChatStore.getState().currentSessionId).toBe(SESSION_A.id);
+    });
+
+    it('does not ping-pong back to the URL hash after switching sessions (#1755)', async () => {
+        vi.useFakeTimers();
+        window.history.replaceState(null, '', `#${getSessionHash(SESSION_A.id)}`);
+
+        await act(async () => {
+            render(<App />);
+        });
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(600);
+        });
+        expect(useChatStore.getState().currentSessionId).toBe(SESSION_A.id);
+
+        // User switches to B (e.g. sidebar click). The hash-sync effect rewrites
+        // the URL hash to B's short hash. The nav guard must NOT re-fire and bounce
+        // back to A — that was the #1755 oscillation.
+        await act(async () => {
+            useChatStore.getState().setCurrentSession(SESSION_B.id);
+        });
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(600);
+        });
+
+        expect(useChatStore.getState().currentSessionId).toBe(SESSION_B.id);
     });
 });

src/gaia/mcp/servers/agent_ui_mcp.py

@@ -74,6 +74,17 @@ def _normalize_error(
     return {"status": "error", "detail": msg}
 
 
+def _is_error(result: Any) -> bool:
+    """True if a tool result is a structured error envelope.
+
+    All boundary errors flow through ``_normalize_error`` which returns
+    ``{"status": "error", ...}``. Callers must key on this shape, not the
+    legacy ``{"error": ...}`` one, or a backend failure silently passes as
+    success.
+    """
+    return isinstance(result, dict) and result.get("status") == "error"
+
+
 def _api(base_url: str, method: str, path: str, **kwargs) -> Dict[str, Any]:
     """Make an API request to the GAIA Agent UI backend."""
     url = f"{base_url}/api{path}"
@@ -291,15 +302,15 @@ def delete_session(session_id: str) -> Dict[str, Any]:
             r.raise_for_status()
             return {"deleted": True, "session_id": session_id}
         except Exception as e:
-            return {"error": str(e)}
+            return _normalize_error(e, backend_url)
 
     # ── Messages ───────────────────────────────────────────────────
 
     @mcp.tool()
     def get_messages(session_id: str) -> Dict[str, Any]:
         """Get all messages in a session (with agent steps and tool outputs)."""
         data = _api(backend_url, "get", f"/sessions/{session_id}/messages")
-        if "error" in data:
+        if _is_error(data):
             return data
         # Simplify for readability
         messages = []
@@ -369,15 +380,16 @@ def index_document(filepath: str, session_id: str = "") -> Dict[str, Any]:
                     f"/sessions/{session_id}/documents",
                     json={"document_id": doc_id},
                 )
-                if "error" not in attach_result:
-                    result["linked_to_session"] = session_id
-                else:
+                if _is_error(attach_result):
                     logger.warning(
                         "Failed to link doc %s to session %s: %s",
                         doc_id,
                         session_id,
-                        attach_result.get("error"),
+                        attach_result.get("detail"),
                     )
+                    result["link_error"] = attach_result.get("detail")
+                else:
+                    result["linked_to_session"] = session_id
         return result
 
     @mcp.tool()
@@ -463,7 +475,10 @@ def take_screenshot(
         try:
             from PIL import Image, ImageGrab
         except ImportError:
-            return {"error": "Pillow not installed. Run: pip install Pillow"}
+            return {
+                "status": "error",
+                "detail": "Pillow not installed. Run: pip install Pillow",
+            }
 
         try:
             bbox = None
@@ -524,7 +539,7 @@ def take_screenshot(
                 "file_size_kb": file_size_kb,
             }
         except Exception as e:
-            return {"error": f"Screenshot failed: {e}"}
+            return {"status": "error", "detail": f"Screenshot failed: {e}"}
 
     # ── Memory ────────────────────────────────────────────────────────
     #

src/gaia/ui/sse_handler.py

@@ -184,17 +184,6 @@ def _emit(self, event: Dict[str, Any]):
         """Push an event to the queue for SSE delivery."""
         self.event_queue.put(event)
 
-    def emit_set_active_session(self, session_id: str) -> None:
-        """Emit a set_active_session event into the per-session chat stream.
-
-        Note: the primary activation path (MCP bridge → App.tsx) routes
-        through _system_emitter in routers/sessions.py, not this method.
-        This is a convenience shim for agent-loop code that already holds
-        an SSEOutputHandler and wants to push the event without importing
-        the sessions router.
-        """
-        self._emit({"type": "set_active_session", "session_id": session_id})
-
     def _elapsed(self) -> float:
         if self._start_time is None:
             return 0.0

tests/unit/test_agent_ui_mcp.py

@@ -138,3 +138,107 @@ def test_docstring_honest(self):
         ), "send_message docstring should not claim real-time streaming"
         # Should mention open_session_in_browser
         assert "open_session_in_browser" in doc
+
+
+class TestIsError:
+    """#1755: error detection must key on the structured envelope, not the
+    legacy ``{"error": ...}`` shape that ``_normalize_error`` replaced."""
+
+    def test_new_envelope_is_error(self):
+        from gaia.mcp.servers.agent_ui_mcp import _is_error
+
+        assert _is_error({"status": "error", "detail": "boom"}) is True
+
+    def test_success_payload_not_error(self):
+        from gaia.mcp.servers.agent_ui_mcp import _is_error
+
+        assert _is_error({"messages": [], "total": 0}) is False
+        assert _is_error({"status": "ok"}) is False
+        assert _is_error("not a dict") is False
+
+    def test_legacy_error_key_not_matched(self):
+        from gaia.mcp.servers.agent_ui_mcp import _is_error
+
+        # The legacy shape no longer appears; _is_error must not depend on it.
+        assert _is_error({"error": "boom"}) is False
+
+
+def _get_tool(name):
+    """Construct the MCP server and return a tool's raw function."""
+    pytest.importorskip("mcp")  # constructing the server needs optional mcp
+    from gaia.mcp.servers.agent_ui_mcp import create_agent_ui_mcp
+
+    with patch("requests.get") as mock_get:
+        mock_get.return_value = MagicMock(
+            status_code=200, json=MagicMock(return_value={})
+        )
+        mock_get.return_value.raise_for_status.return_value = None
+        mcp = create_agent_ui_mcp("http://localhost:4200")
+
+    tool = mcp._tool_manager._tools.get(name)  # pylint: disable=protected-access
+    if tool is None:
+        pytest.skip(f"{name} tool not found in tool manager")
+    return tool.fn
+
+
+class TestGetMessagesSurfacesError:
+    """#1755: get_messages keyed on the old shape, so a backend error fell
+    through to an empty success payload (silent fallback)."""
+
+    def test_backend_error_not_silent_empty(self):
+        get_messages = _get_tool("get_messages")
+
+        mock_resp = MagicMock()
+        mock_resp.status_code = 404
+        mock_resp.json.return_value = {"detail": "Session not found"}
+        mock_resp.raise_for_status.side_effect = requests.exceptions.HTTPError(
+            response=mock_resp
+        )
+        with patch("requests.get", return_value=mock_resp):
+            result = get_messages("nonexistent")
+
+        assert result["status"] == "error"
+        assert result.get("detail")
+        # Regression: must NOT degrade to {"messages": [], "total": 0}.
+        assert "messages" not in result
+
+
+class TestIndexDocumentLinkFailure:
+    """#1755: index_document reported a session link succeeded even when the
+    attach call failed (false success)."""
+
+    @staticmethod
+    def _upload_ok():
+        resp = MagicMock()
+        resp.status_code = 200
+        resp.json.return_value = {"id": "doc-1"}
+        resp.raise_for_status.return_value = None
+        return resp
+
+    def test_attach_failure_not_reported_as_linked(self):
+        index_document = _get_tool("index_document")
+
+        attach_resp = MagicMock()
+        attach_resp.status_code = 404
+        attach_resp.json.return_value = {"detail": "Session not found"}
+        attach_resp.raise_for_status.side_effect = requests.exceptions.HTTPError(
+            response=attach_resp
+        )
+        with patch("requests.post", side_effect=[self._upload_ok(), attach_resp]):
+            result = index_document("/tmp/file.pdf", session_id="sess-1")
+
+        assert "linked_to_session" not in result
+        assert result.get("link_error")
+
+    def test_attach_success_reports_linked(self):
+        index_document = _get_tool("index_document")
+
+        attach_resp = MagicMock()
+        attach_resp.status_code = 200
+        attach_resp.json.return_value = {"attached": True}
+        attach_resp.raise_for_status.return_value = None
+        with patch("requests.post", side_effect=[self._upload_ok(), attach_resp]):
+            result = index_document("/tmp/file.pdf", session_id="sess-1")
+
+        assert result.get("linked_to_session") == "sess-1"
+        assert "link_error" not in result