fix(mcp): surface backend errors in Agent UI MCP callers + stop session nav oscillation#1758
Draft
github-actions[bot] wants to merge 1 commit into
Draft
fix(mcp): surface backend errors in Agent UI MCP callers + stop session nav oscillation#1758github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
…on nav oscillation The #1750 envelope change switched boundary errors to {"status":"error"}, but get_messages and index_document still keyed on the legacy {"error"} shape — so a backend failure (e.g. 404) silently fell through to an empty {"messages":[],"total":0} payload, and a failed session-document attach was reported as linked. Both now detect the structured envelope via _is_error and fail loudly. delete_session and the screenshot tool are normalized to the same envelope, and the unused emit_set_active_session shim is removed. App.tsx: the URL-hash nav guard depended on currentSessionId, so it re-fired on every session switch, read a stale short hash, and scheduled a ~500ms bounce that could ping-pong A<->B — breaking the MCP->UI activation flow #1750 added. The guard now runs once on mount and reads currentSessionId fresh inside the timer. Replaces the tautological App.test.tsx with a real render-based regression test for the guard. Closes #1755
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The #1750 review landed several findings after merge, so these regressions are currently on
main. Two of them violate the "No Silent Fallbacks" rule: after #1750 switched MCP boundary errors to the{"status":"error","detail":...}envelope,get_messagesandindex_documentstill keyed on the old{"error"}shape — so a backend failure (e.g. a 404 for a missing session) silently fell through to an empty{"messages":[],"total":0}payload, and a failed session-document attach was reported as a successful link. Both now detect the envelope and fail loudly. The third is a 🔴 UI regression: theApp.tsxURL-hash nav guard re-ran on everycurrentSessionIdchange, read a stale 7-char short hash (which never equals the full UUID), and scheduled a ~500ms bounce that could ping-pong sessions — breaking the very MCP→UI activation flow #1750 added. The guard now runs once on mount.Also folds in the review's minor items:
delete_sessionand the screenshot tool now use the same structured envelope, and the unusedemit_set_active_sessionshim is removed. The tautologicalApp.test.tsx(literal-vs-literal assertions that never renderedApp) is replaced with a real render-based regression test for the nav guard.Closes #1755
Test plan
python -m pytest tests/unit/test_agent_ui_mcp.py— 13 passed (incl. newget_messagessilent-fallback andindex_documentfalse-success regression tests)python -m pytest tests/unit/— 5231 passed (1 pre-existing, unrelatedtest_memory_routerFAISS failure also present onmain)python util/lint.py --flake8/--black/--isortcleancd src/gaia/apps/webui && npm run test— runs the newApp.test.tsxnav-guard test (vitest; node_modules not available in this job)