fix(code): wrap MCP viewer navigation (#4677)

MCP viewer keyboard navigation now wraps at list boundaries, with arrow
keys moving through every row and `Tab` / `Shift+Tab` moving between
servers.

---

The MCP viewer now lets users continue navigating at list boundaries
instead of reversing direction.

`Up` and `Down` wrap through every displayed row, including server
headers and tools. `Tab` and `Shift+Tab` provide faster server-level
navigation by moving to the next or previous server header and wrapping
at either end. From a tool, `Shift+Tab` returns to that tool's server
header, while `Tab` advances to the next server.

This also makes header-only lists—such as when every server has an
error—fully navigable with either set of keys.
This commit is contained in:
Mason Daugherty
2026-07-13 09:31:39 -04:00
committed by GitHub
parent b2d62ce456
commit cffc73213b
4 changed files with 202 additions and 82 deletions
+1 -1
View File
@@ -13193,7 +13193,7 @@ class DeepAgentsApp(App):
self.screen.action_move_up()
return
if isinstance(self.screen, MCPViewerScreen):
self.screen.action_move_up()
self.screen.action_jump_up()
return
# shift+tab is reused for navigation inside modal screens (e.g.
# ModelSelectorScreen); skip the toggle so it doesn't fire through.
@@ -1345,10 +1345,10 @@ class MCPViewerScreen(ModalScreen[str | None]):
# bottom for up).
def _move_selection(self, delta: int) -> None:
"""Move selection by delta row positions, clamped at the list ends.
"""Move selection by delta row positions within the list bounds.
No wrap-around — pressing `Down` past the last row stays put rather
than jumping to the first. Walks every row (headers + tools).
Walks every row (headers + tools). Navigation actions handle wrapping
before calling this helper at a list boundary.
Args:
delta: Number of row positions to move.
@@ -1359,22 +1359,22 @@ class MCPViewerScreen(ModalScreen[str | None]):
if 0 <= target < len(self._row_widgets):
self._move_to(target)
def _next_tool_row(self, start: int, step: int) -> int | None:
"""Return the index of the next `MCPToolItem` row in `step` direction.
Used by `Tab` / `Shift+Tab` to skip server-header rows during
cross-tool navigation. Returns `None` when there is no tool row in
the requested direction.
def _next_server_header(self, start: int, step: int) -> int | None:
"""Return the next server-header index in the requested direction.
Args:
start: Index to start searching from (exclusive).
step: `+1` (forward) or `-1` (backward).
Returns:
The index of the nearest `MCPServerHeaderItem` in that direction,
or `None` when no server header exists there.
"""
idx = start + step
while 0 <= idx < len(self._row_widgets):
if isinstance(self._row_widgets[idx], MCPToolItem):
return idx
idx += step
index = start + step
while 0 <= index < len(self._row_widgets):
if isinstance(self._row_widgets[index], MCPServerHeaderItem):
return index
index += step
return None
def _scroll_widget_bottom_to_view(
@@ -1426,11 +1426,11 @@ class MCPViewerScreen(ModalScreen[str | None]):
"""Smart up: scroll one row inside a tall expanded row, else jump.
If the selected row's top edge is already inside the viewport, jump
to the previous row (header or tool). For rows taller than the
viewport, pin the new selection's **bottom** to the viewport so the
next `Up` resumes line-stepping through that row; otherwise just
ensure the row is visible. `Tab` / `Shift+Tab` skip the smart check
AND skip header rows (see `action_jump_up`).
to the previous row (header or tool), wrapping to the final row from
the first. For rows taller than the viewport, pin the new selection's
**bottom** to the viewport so the next `Up` resumes line-stepping
through that row; otherwise just ensure the row is visible. `Tab` /
`Shift+Tab` jump between server headers (see `action_jump_up`).
"""
if not self._row_widgets:
return
@@ -1438,7 +1438,10 @@ class MCPViewerScreen(ModalScreen[str | None]):
selected = self._row_widgets[self._selected_index]
if selected.region.y >= scroll.region.y:
old = self._selected_index
self._move_selection(-1)
if old == 0:
self._move_to(len(self._row_widgets) - 1)
else:
self._move_selection(-1)
if self._selected_index != old:
self._reveal_selection(
self._row_widgets[self._selected_index], direction=-1
@@ -1450,10 +1453,11 @@ class MCPViewerScreen(ModalScreen[str | None]):
"""Smart down: scroll one row inside a tall expanded row, else jump.
If the selected row's bottom edge is already inside the viewport,
jump to the next row (header or tool). For rows taller than the
viewport, pin the new selection's top to the viewport; otherwise
just ensure the row is visible. `Tab` / `Shift+Tab` skip the smart
check AND skip header rows (see `action_jump_down`).
jump to the next row (header or tool), wrapping to the first row from
the final one. For rows taller than the viewport, pin the new
selection's top to the viewport; otherwise just ensure the row is
visible. `Tab` / `Shift+Tab` jump between server headers (see
`action_jump_down`).
"""
if not self._row_widgets:
return
@@ -1463,7 +1467,10 @@ class MCPViewerScreen(ModalScreen[str | None]):
viewport_bottom = scroll.region.y + scroll.region.height
if selected_bottom <= viewport_bottom:
old = self._selected_index
self._move_selection(1)
if old == len(self._row_widgets) - 1:
self._move_to(0)
else:
self._move_selection(1)
if self._selected_index != old:
self._reveal_selection(
self._row_widgets[self._selected_index], direction=1
@@ -1472,17 +1479,26 @@ class MCPViewerScreen(ModalScreen[str | None]):
scroll.scroll_relative(y=1, animate=False)
def action_jump_up(self) -> None:
"""Jump to the previous tool (Shift+Tab); skips headers."""
target = self._next_tool_row(self._selected_index, -1)
"""Jump backward to the nearest server header (Shift+Tab), wrapping.
From a tool row this lands on the current server's own header; from a
header it moves to the previous server. Wraps to the final header from
the top.
"""
target = self._next_server_header(self._selected_index, -1)
if target is None:
target = self._next_server_header(len(self._row_widgets), -1)
if target is None or target == self._selected_index:
return
self._move_to(target)
self._reveal_selection(self._row_widgets[target], direction=-1)
def action_jump_down(self) -> None:
"""Jump to the next tool (Tab); skips headers."""
target = self._next_tool_row(self._selected_index, +1)
"""Jump to the next server (Tab), wrapping at the end."""
target = self._next_server_header(self._selected_index, +1)
if target is None:
target = self._next_server_header(-1, +1)
if target is None or target == self._selected_index:
return
self._move_to(target)
self._reveal_selection(self._row_widgets[target], direction=1)
+30
View File
@@ -17925,6 +17925,36 @@ class TestNotificationCenterIntegration:
assert screen._selected != start
assert app._auto_approve is False
async def test_mcp_viewer_shift_tab_jumps_to_previous_server(self) -> None:
"""App-level shift+tab routes to MCPViewerScreen.jump_up."""
from deepagents_code.mcp_tools import MCPServerInfo, MCPToolInfo
from deepagents_code.tui.widgets.mcp_viewer import MCPViewerScreen
servers = [
MCPServerInfo(
name="first",
transport="stdio",
tools=(MCPToolInfo(name="first-tool", description=""),),
),
MCPServerInfo(
name="second",
transport="stdio",
tools=(MCPToolInfo(name="second-tool", description=""),),
),
]
app = DeepAgentsApp(agent=MagicMock(), thread_id="t")
async with app.run_test() as pilot:
await pilot.pause()
screen = MCPViewerScreen(server_info=servers)
app.push_screen(screen)
await pilot.pause()
assert screen._selected_index == 0
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 2
assert app._auto_approve is False
async def test_toast_identity_warn_once_semantics(
self, caplog: pytest.LogCaptureFixture
) -> None:
@@ -709,12 +709,10 @@ class TestMCPViewerScreen:
assert "1 tool)" in text
async def test_keyboard_navigation(self) -> None:
"""Arrow / Tab navigation walks rows (headers + tools).
"""Arrows walk every row while Tab moves between servers.
Up/Down step through every row including server headers. Tab /
Shift+Tab skip headers as a faster cross-tool nav. Vim-style
`j`/`k` bindings are absent so they can be typed into the filter
Input — see `test_letter_keys_type_into_filter`.
Vim-style `j`/`k` bindings are absent so they can be typed into the
filter Input — see `test_letter_keys_type_into_filter`.
"""
from textual.widgets import Input
@@ -741,42 +739,40 @@ class TestMCPViewerScreen:
await pilot.pause()
assert screen._selected_index == expected
# No wrap-around at the end of the list.
# Down from the final row wraps to the first row.
await pilot.press("down")
await pilot.pause()
assert screen._selected_index == 4
assert screen._selected_index == 0
# Up walks back row by row.
# Up from the first row wraps back to the final row, then walks
# backward row by row.
await pilot.press("up")
await pilot.pause()
assert screen._selected_index == 4
await pilot.press("up")
await pilot.pause()
assert screen._selected_index == 3
# Tab skips headers — from row 3 (remote-api header), next tool
# is row 4 (search).
# Tab wraps from the current row (3) to the first server header,
# then advances to the next server header.
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 4
# Tab from search: no further tool → no-op.
assert screen._selected_index == 0
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 3
# From a tool (4), Shift+Tab returns to its own server header (3),
# then moves to the previous server header (0).
await pilot.press("down")
await pilot.pause()
assert screen._selected_index == 4
# Shift+Tab skip-header: search (4) → write_file (2).
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 2
# Shift+Tab again: write_file (2) → read_file (1).
assert screen._selected_index == 3
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 1
# Shift+Tab from read_file (1): no prior tool (only header
# before) → no-op.
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 1
assert screen._selected_index == 0
# The filter Input exists and is the focused widget.
filter_input = screen.query_one("#mcp-filter", Input)
@@ -1241,8 +1237,8 @@ class TestMCPViewerScreen:
assert screen._selected_index == 1
assert scroll.scroll_offset.y < offset_before
async def test_no_wrap_around_at_list_ends(self) -> None:
"""Down past the last row, or Up past the first, are no-ops."""
async def test_navigation_wraps_at_list_ends(self) -> None:
"""Arrow and Tab navigation wrap in both directions."""
# Rows: [0: filesystem header, 1: read_file, 2: write_file,
# 3: remote-api header, 4: search]
app = MCPViewerTestApp()
@@ -1252,33 +1248,28 @@ class TestMCPViewerScreen:
await pilot.pause()
assert screen._selected_index == 0
# Up from the first row (header) stays put.
# Up from the first row wraps to the final row; Down wraps back.
await pilot.press("up")
await pilot.pause()
assert screen._selected_index == 0
# Shift+Tab from a header with no prior tool is also a no-op.
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 0
# Walk to the last row (search, row 4) via Down.
for _ in range(4):
await pilot.press("down")
await pilot.pause()
assert screen._selected_index == 4
# Down past the last row stays put.
await pilot.press("down")
await pilot.pause()
assert screen._selected_index == 4
assert screen._selected_index == 0
# Tab past the last tool also stays put.
# Shift+Tab from the first server wraps to the final server; Tab
# from there wraps back to the first server.
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 3
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 4
assert screen._selected_index == 0
async def test_tab_always_jumps_even_inside_tall_tool(self) -> None:
"""Tab / Shift+Tab unconditionally jump, ignoring viewport visibility."""
"""Tab / Shift+Tab jump to headers despite tall expanded content."""
from textual.containers import VerticalScroll
long_desc = "\n".join(f"line {i}" for i in range(40))
info = [
MCPServerInfo(
@@ -1289,30 +1280,104 @@ class TestMCPViewerScreen:
MCPToolInfo(name="next", description="short"),
),
),
MCPServerInfo(
name="other",
transport="stdio",
tools=(MCPToolInfo(name="last", description="short"),),
),
]
# Rows: [0: srv header, 1: big, 2: next]
# Rows: [0: srv header, 1: big, 2: next, 3: other header, 4: last]
app = MCPViewerTestApp()
async with app.run_test() as pilot:
screen = MCPViewerScreen(server_info=info)
app.push_screen(screen)
await pilot.pause()
# Step from header to big and expand it.
# Step from the header to big and expand it.
await pilot.press("down")
await pilot.press("enter")
await pilot.pause()
assert screen._selected_index == 1
# One Tab must jump to "next" (row 2) even though Down would
# only scroll one row.
# Precondition: big is now taller than the viewport, so a plain
# Down would only line-scroll — the jump below must ignore that.
scroll = screen.query_one(".mcp-list", VerticalScroll)
assert screen._row_widgets[1].region.height > scroll.region.height
# Tab jumps past the expanded body to the next server.
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 2
assert screen._selected_index == 3
# Shift+Tab back to "big" (row 1).
# Return to big, then Shift+Tab jumps to its server header.
await pilot.press("shift+tab")
await pilot.press("down")
await pilot.pause()
assert screen._selected_index == 1
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 0
async def test_tab_single_server_returns_to_header(self) -> None:
"""Tab/Shift+Tab return a tool to its sole header, else no-op.
With one server, jumping from the tool lands on that server's header.
Jumping again from the lone header has nowhere to go, so it must be a
true no-op that leaves the scroll offset untouched — even when an
expanded tool has scrolled the viewport.
"""
from textual.containers import VerticalScroll
long_desc = "\n".join(f"line {i}" for i in range(40))
info = [
MCPServerInfo(
name="srv",
transport="stdio",
tools=(MCPToolInfo(name="only", description=long_desc),),
),
]
# Rows: [0: srv header, 1: only]
app = MCPViewerTestApp()
async with app.run_test() as pilot:
screen = MCPViewerScreen(server_info=info)
app.push_screen(screen)
await pilot.pause()
# Tab and Shift+Tab from the tool both return to the sole header.
await pilot.press("down")
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 0
await pilot.press("down")
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 0
# Expand the tool (taller than the viewport) and Tab back up to the
# header so a later scroll has room to move.
await pilot.press("down")
await pilot.press("enter")
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 0
scroll = screen.query_one(".mcp-list", VerticalScroll)
scroll.scroll_relative(y=10, animate=False)
await pilot.pause()
offset = scroll.scroll_offset
# On the lone header, Tab and Shift+Tab are no-ops: neither the
# selection nor the viewport moves.
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 0
assert scroll.scroll_offset == offset
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 0
assert scroll.scroll_offset == offset
async def test_server_header_rows_are_selectable(self) -> None:
"""Up/Down lands the cursor on server header rows too (R10)."""
@@ -1370,13 +1435,22 @@ class TestMCPViewerScreen:
await pilot.pause()
assert screen._selected_index == 1
# No wrap; no tools to Tab to.
# Down wraps across header-only lists. Tab and Shift+Tab traverse
# those same rows even though there are no tools.
await pilot.press("down")
await pilot.pause()
assert screen._selected_index == 1
assert screen._selected_index == 0
await pilot.press("tab")
await pilot.pause()
assert screen._selected_index == 1
await pilot.press("shift+tab")
await pilot.pause()
assert screen._selected_index == 0
# Up wraps from the first header back to the last.
await pilot.press("up")
await pilot.pause()
assert screen._selected_index == 1
async def test_enter_on_unauth_header_dismisses_with_server_name(self) -> None:
"""Activating an unauthenticated header returns the server name.