mirror of
https://github.com/langchain-ai/deepagents.git
synced 2026-07-21 17:25:26 -04:00
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:
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user