Fix mcp permissions render clipping (#1424)

.github/workflows/smoke.yml

@@ -176,7 +176,7 @@ jobs:
     name: Screenshot Regression (Linux)
     needs: publish
     runs-on: ubuntu-latest
-    timeout-minutes: 30
+    timeout-minutes: 45
 
     env:
       SMOKE_OLLAMA_MODEL: qwen2:0.5b

.github/workflows/validate_docker_image.yml

@@ -46,7 +46,7 @@ jobs:
   validate-docker-build:
     name: Validate Docker Build
     runs-on: ubuntu-latest
-    timeout-minutes: 20
+    timeout-minutes: 30
 
     steps:
       - name: "Checkout"

scripts/smoke/run-native-tape.sh

@@ -116,19 +116,16 @@ collect_failure_artifacts() {
   echo "    artifacts: $(ls "${ARTIFACT_DIR}" 2>/dev/null | tr '\n' ' ')"
 }
 
-# Substitute placeholders in preamble. Sed delimiter is '|' since paths
-# contain '/'. The substituted values are paths set by us, so escaping
-# is minimal.
-sed \
+# Substitute placeholders in both preamble and body. Sed delimiter is '|'
+# since paths contain '/'. Concatenating both files before the sed pass
+# means body tapes can use any token, not just the preamble.
+cat "$preamble" "$body" | sed \
   -e "s|__NETCLAW_HOME__|${NETCLAW_HOME}|g" \
   -e "s|__NETCLAW_BIN_DIR__|${NETCLAW_BIN_DIR}|g" \
   -e "s|__NETCLAW_DAEMON__|${NETCLAW_SMOKE_DAEMON}|g" \
   -e "s|__TAPE_NAME__|${TAPE_NAME}|g" \
-  "$preamble" > "$combined"
-
-# Append body. The body declares its own `Output ...`; last-write-wins
-# on Output is fine in vhs.
-cat "$body" >> "$combined"
+  -e "s|__NETCLAW_SMOKE_MCP_SERVER__|${NETCLAW_SMOKE_MCP_SERVER:-}|g" \
+  > "$combined"
 
 echo "==> Running native tape: ${TAPE_NAME} (timeout=${TAPE_TIMEOUT_S}s)"
 echo "    NETCLAW_BIN_DIR=${NETCLAW_BIN_DIR}"

scripts/smoke/run-smoke.sh

@@ -73,12 +73,14 @@ FULL_SCENARIOS=("${LIGHT_SCENARIOS[@]}")
 # may emit several `Screenshot "/tmp/shot-<frame>.png"` directives. SHOT_FRAMES
 # is the full set of frame names the harness compares against baselines — it
 # MUST stay in sync with the Screenshot paths in those tapes.
-SHOT_TAPES=(help wizard-screens provider-manager)
+SHOT_TAPES=(help wizard-screens provider-manager mcp-permissions)
 SHOT_FRAMES=(
   help
   wizard-provider-picker
   wizard-security-posture
   provider-manager-empty
+  mcp-permissions-server-list
+  mcp-permissions-tool-grid
 )
 
 usage() {

src/Netclaw.Cli.Tests/Mcp/McpToolPermissionsPageTests.cs

@@ -300,6 +300,34 @@ public async Task ToolGrid_RightArrowOnServerEnabledRow_TogglesServerAccess()
         Assert.NotEqual(wasBefore, vm.IsServerAllowedForSelectedAudience());
     }
 
+    [Fact]
+    public async Task ToolGrid_ManyTools_HeaderRowsNotOverwrittenByScrollContent()
+    {
+        // Regression test for issue #1424: ScrollableContainerNode.Render ignores
+        // bounds.Y and writes at context (0,0). With enough tools the scroll container
+        // overwrites the server-info and audience rows. The fix wraps the container in
+        // a borderless PanelNode so it receives a properly-offset render context.
+        var (terminal, app, vm) = CreateHeadlessApp(out var input);
+
+        // 15 tools: on a 40-row terminal the tool list starts at row ~10 without the
+        // bug fix; with the bug it writes at row 0 and overwrites every header row.
+        var tools = Enumerable.Range(1, 15).Select(i => $"tool-{i:00}").ToList();
+        vm.InitializeForTests(new McpServerName("notion"), tools);
+        vm.SetSelectedAudienceForTests(TrustAudience.Personal);
+
+        input.EnqueueKey(ConsoleKey.Q, false, false, true);
+
+        using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
+        await app.RunAsync(cts.Token);
+
+        Assert.True(terminal.Contains("MCP Permissions"),
+            $"Expected page header. Screen:\n{terminal}");
+        Assert.True(terminal.Contains("Audience"),
+            $"Expected 'Audience' row not overwritten by tool list. Screen:\n{terminal}");
+        Assert.True(terminal.Contains("Server default"),
+            $"Expected 'Server default' row not overwritten by tool list. Screen:\n{terminal}");
+    }
+
     // ── Helpers ──────────────────────────────────────────────────────────────
 
     private (VirtualTerminal Terminal, TerminaApplication App, McpToolPermissionsViewModel Vm)

src/Netclaw.Cli/Mcp/McpToolPermissionsPage.cs

@@ -177,7 +177,15 @@ private ILayoutNode BuildToolGrid()
             .WithAutoScroll(AutoScrollPolicy.None)
             .WithContent(_toolRowsNode);
         _toolScrollNode.Fill();
-        layout = layout.WithChild(_toolScrollNode);
+        // ScrollableContainerNode.Render ignores bounds.Y and writes relative to the
+        // context's (0,0). Hosting it inside a borderless PanelNode ensures the
+        // PanelNode calls context.CreateSubContext(bounds) first, so the scroll
+        // container receives a context already offset to its actual screen position.
+        layout = layout.WithChild(
+            new PanelNode()
+                .WithBorder(BorderStyle.None)
+                .WithContent(_toolScrollNode)
+                .Fill());
 
         return layout;
     }
@@ -226,7 +234,15 @@ private void EnsureToolCursorVisible()
     {
         if (_toolScrollNode is null || _gridCursor < FirstToolRow) return;
         var toolIdx = _gridCursor - FirstToolRow;
-        if (_toolScrollNode.MaxScroll == 0) return;
+        if (_toolScrollNode.MaxScroll == 0)
+        {
+            // All tools fit in the viewport. Reset any stale offset left over from
+            // a prior larger scroll position (e.g. after a terminal resize or after
+            // the audience changes to a smaller visible tool set).
+            if (_toolScrollNode.ScrollOffset != 0)
+                _toolScrollNode.ScrollTo(0);
+            return;
+        }
         var viewportH = _toolScrollNode.ContentHeight - _toolScrollNode.MaxScroll;
         if (viewportH <= 0) return;
         if (toolIdx < _toolScrollNode.ScrollOffset)
@@ -249,7 +265,7 @@ private LayoutNode BuildFooter()
             if (_confirmingSave)
             {
                 return new TextNode("Save changes?  [Y] Yes  [N] No  [Esc] Cancel")
-                    .WithForeground(Color.Yellow).Bold();
+                    .WithForeground(Color.Yellow).Bold().NoWrap();
             }
 
             var hints = ViewModel.CurrentState.Value switch
@@ -266,28 +282,14 @@ private LayoutNode BuildFooter()
                 var hasStatus = !string.IsNullOrEmpty(statusText);
 
                 if (ViewModel.HasSaveError)
-                {
-                    return Layouts.Horizontal()
-                        .WithChild(new TextNode(hints).WithForeground(Color.BrightBlack))
-                        .WithChild(new TextNode($"  {statusText}").WithForeground(Color.Red));
-                }
-
+                    return BuildToolGridFooterWithStatus(hints, $"  {statusText}", Color.Red);
                 if (ViewModel.HasUnsavedChanges)
-                {
-                    return Layouts.Horizontal()
-                        .WithChild(new TextNode(hints).WithForeground(Color.BrightBlack))
-                        .WithChild(new TextNode("  *unsaved*").WithForeground(Color.Yellow));
-                }
-
+                    return BuildToolGridFooterWithStatus(hints, "  *unsaved*", Color.Yellow);
                 if (hasStatus)
-                {
-                    return Layouts.Horizontal()
-                        .WithChild(new TextNode(hints).WithForeground(Color.BrightBlack))
-                        .WithChild(new TextNode($"  {statusText}").WithForeground(Color.Green));
-                }
+                    return BuildToolGridFooterWithStatus(hints, $"  {statusText}", Color.Green);
             }
 
-            return new TextNode(hints).WithForeground(Color.BrightBlack);
+            return new TextNode(hints).WithForeground(Color.BrightBlack).NoWrap();
         });
 
         ViewModel.StateVersion
@@ -297,6 +299,13 @@ private LayoutNode BuildFooter()
         return _footerNode;
     }
 
+    private static LayoutNode BuildToolGridFooterWithStatus(string hints, string status, Color color)
+    {
+        return Layouts.Horizontal()
+            .WithChild(new TextNode(hints).WithForeground(Color.BrightBlack).NoWrap())
+            .WithChild(new TextNode(status).WithForeground(color).WidthAuto());
+    }
+
     private void HandleKeyPress(KeyPressed key)
     {
         var keyInfo = key.KeyInfo;

tests/smoke/tapes/screenshots/mcp-permissions.tape

@@ -0,0 +1,88 @@
+# mcp-permissions.tape (screenshot) — capture `netclaw mcp permissions` with
+# the smoke MCP server (smoke-math) connected and its tools indexed.
+#
+# Capture-only tape: NO post-tape assertion. The screenshots mode in
+# run-smoke.sh compares each emitted PNG byte-for-byte against the committed
+# baseline at tests/smoke/screenshots/<frame>.approved.png.
+#
+# Frames captured:
+#   mcp-permissions-server-list — ServerList state: smoke-math appears as
+#                                 "Connected, 3 tools" below the header.
+#   mcp-permissions-tool-grid  — ToolGrid state: header rows (Audience,
+#                                Server enabled, Server default) and all
+#                                three tool rows (add, echo, record-tasks)
+#                                simultaneously visible. This is the direct
+#                                regression check for issue #1424 — the
+#                                scroll container must not overwrite the header.
+#
+# Why these frames matter: the Loading/no-daemon frame only confirmed the
+# header layout compiles. These frames confirm the layout holds under real
+# data: a connected server, its tool rows rendered at the correct vertical
+# offset, and all metadata rows above them untouched.
+#
+# Prepended preamble: tapes/screenshot-preamble.tape (determinism-pinned).
+# __NETCLAW_SMOKE_MCP_SERVER__ is substituted by run-native-tape.sh.
+
+Output "/tmp/tape-shot-mcp-permissions.gif"
+
+# ─── Setup: seed provider + register the smoke MCP server ────────────
+# The smoke Ollama instance is already running at 127.0.0.1:11434 in
+# screenshots mode. We seed a minimal provider so the daemon starts cleanly.
+Hide
+Type "netclaw provider add smoke-llm ollama --endpoint http://127.0.0.1:11434"
+Enter
+Wait+Screen@10s /TAPE\$/
+
+Type "netclaw model set main smoke-llm qwen2:0.5b"
+Enter
+Wait+Screen@10s /TAPE\$/
+
+# Register the deterministic smoke MCP server (add, echo, record-tasks).
+# --grant-all means every tool is auto-approved for all audiences.
+Type "netclaw mcp add --transport stdio --grant-all smoke-math -- __NETCLAW_SMOKE_MCP_SERVER__"
+Enter
+Wait+Screen@10s /TAPE\$/
+
+# Start the daemon (detaches immediately; MCP handshake follows async).
+Type "netclaw daemon start"
+Enter
+Wait+Screen@20s /TAPE\$/
+
+# Allow time for the daemon to spawn the stdio MCP process and index tools.
+Sleep 5s
+Show
+
+# ─── Launch the TUI ──────────────────────────────────────────────────
+Type "netclaw mcp permissions"
+Enter
+
+# ─── Frame 1: ServerList ─────────────────────────────────────────────
+# smoke-math should appear as "Connected, 3 tools".
+Wait+Screen@15s /smoke-math/
+Sleep 1s
+Screenshot "/tmp/shot-mcp-permissions-server-list.png"
+Sleep 1s
+
+# ─── Navigate into the ToolGrid ──────────────────────────────────────
+Enter
+
+# ─── Frame 2: ToolGrid ───────────────────────────────────────────────
+# All header rows (Server, Audience, Server enabled, Server default) plus
+# all tool rows (add, echo, record-tasks) must be visible simultaneously.
+# If the #1424 regression reappears, tool rows will overwrite the header.
+Wait+Screen@10s /record-tasks/
+Sleep 1s
+Screenshot "/tmp/shot-mcp-permissions-tool-grid.png"
+Sleep 1s
+
+# ─── Exit TUI ────────────────────────────────────────────────────────
+Ctrl+Q
+Wait+Screen@10s /TAPE\$/
+
+# Stop the daemon so the tape-level cleanup does not race the next tape.
+Type "netclaw daemon stop"
+Enter
+Wait+Screen@10s /TAPE\$/
+
+Type "exit"
+Enter