fix(tui): show MCP servers that failed to start in /mcp

[Vasanthdev2004]

Fixes #825. Companion to #822, which fixed the same blind spot in zero mcp check.

/mcp worked out each server's state from the config file — disabled if you turned it off, enabled otherwise. But MCP registration is best-effort: a server that can't be reached gets recorded and startup carries on. So a server that never connected showed up as enabled, its tools quietly missing, and nothing in the panel said why.

Startup does know — it prints a warning per skipped server to stderr. That's gone by the time you notice, and /mcp is exactly where you go afterwards to ask what's actually running.

So the skipped set now reaches the panel, and a server that failed renders as failed with the reason under it:

› docs · failed · stdio
  exec: "docs-mcp": executable file not found in $PATH

Two details worth calling out:

The reason comes from the server, so it goes through redaction.ErrorMessage before it's rendered. A handshake error that echoes the Authorization header back would otherwise print the bearer token straight into the transcript. There's a test for that.

Disabled wins over failed. If you turned a server off it was never expected to connect, and calling it failed would be misleading.

The stderr warning is unchanged — non-interactive users still get it, and the panel is an addition rather than a replacement.

Still not fixed, and out of scope here: enabling a server from inside the TUI updates the config but doesn't reconnect anything, so it'll show as enabled while not actually running until you restart. That's pre-existing and a bigger change; happy to file it separately if you'd like.

Verified with mutation testing — eight mutations across the state builder, the renderer, and both wiring points, all killed. TestAltScreenTranscriptScrollKeepsFooterFixed and TestBuildServeScopeKeepsLexicalPaths fail on my Windows box on clean main too (the second needs symlink privilege).

Summary by CodeRabbit

• New Features

  • MCP servers that fail to start are now shown as failed in the /mcp panel.
  • Failure reasons are displayed when available, with sensitive information removed.
  • Servers with missing error details now show a fallback explanation.

• Bug Fixes

  • MCP status now reflects actual startup results rather than configuration alone.
  • Disabled servers continue to appear as disabled, even if startup failures were recorded.

[coderabbitai]

Walkthrough

MCP startup failures now flow from the CLI runtime into TUI options and model state. The /mcp panel maps skipped servers to a failed state, displays redacted or fallback reasons, and includes CLI, state-building, rendering, and end-to-end tests.

Changes

MCP failure visibility

Layer / File(s)	Summary
Runtime failure handoff internal/cli/app.go, internal/cli/app_mcp_skipped_test.go, internal/tui/options.go, internal/tui/model.go	The CLI passes mcpRuntime.Skipped() through tui.Options; the TUI stores it in model state, with coverage for the handoff and stderr warning.
MCP failure state and rendering internal/tui/mcp_state.go, internal/tui/mcp_view.go, internal/tui/command_views.go, internal/tui/mcp_failed_state_test.go	Skipped servers become failed, disabled servers retain precedence, failure messages are redacted or defaulted, and the MCP panel renders the status and reason with state, rendering, and end-to-end coverage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPRuntime
  participant CLI
  participant TUIModel
  participant MCPView
  MCPRuntime->>CLI: Return skipped server failures
  CLI->>TUIModel: Pass MCPSkipped through tui.Options
  TUIModel->>MCPView: Build MCP view state
  MCPView->>MCPView: Render failed status and redacted reason

Loading

Suggested reviewers: gnanam1990, anandh8x, pierrunoyt

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Concise and specific; it matches the main change of surfacing failed MCP servers in /mcp.
Linked Issues check	✅ Passed	The PR threads Runtime.Skipped into TUI state and renders a third failed state with a reason, matching #825.
Out of Scope Changes check	✅ Passed	The changes stay focused on /mcp state wiring, rendering, and tests for failed MCP servers.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/825-mcp-panel-shows-failures

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/cli/app_mcp_skipped_test.go (1)

63-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that the failure reason is forwarded.

The test only verifies the server name. Also assert MCPSkipped[0].Err contains "connection refused" so a regression that drops the recorded error cannot pass.

Proposed test strengthening

 if len(launchedOptions.MCPSkipped) != 1 ||
-	launchedOptions.MCPSkipped[0].Name != "docs" {
+	launchedOptions.MCPSkipped[0].Name != "docs" ||
+	launchedOptions.MCPSkipped[0].Err == nil ||
+	launchedOptions.MCPSkipped[0].Err.Error() != "connection refused" {
 	t.Fatalf("MCPSkipped = %#v, want the failure startup recorded", launchedOptions.MCPSkipped)
 }

As per coding guidelines, add a regression test for behavior changes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/cli/app_mcp_skipped_test.go` around lines 63 - 66, Strengthen the
existing MCPSkipped assertion in the test by also verifying that
MCPSkipped[0].Err contains “connection refused,” while preserving the current
server-name check and failure-count validation.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/cli/app_mcp_skipped_test.go`:
- Around line 63-66: Strengthen the existing MCPSkipped assertion in the test by
also verifying that MCPSkipped[0].Err contains “connection refused,” while
preserving the current server-name check and failure-count validation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6847c24c-e809-446d-80a2-a6db7325507d

📥 Commits

Reviewing files that changed from the base of the PR and between 097c265 and be58076.

📒 Files selected for processing (8)

• internal/cli/app.go
• internal/cli/app_mcp_skipped_test.go
• internal/tui/command_views.go
• internal/tui/mcp_failed_state_test.go
• internal/tui/mcp_state.go
• internal/tui/mcp_view.go
• internal/tui/model.go
• internal/tui/options.go

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: be580766672c
Changed files (8): internal/cli/app.go, internal/cli/app_mcp_skipped_test.go, internal/tui/command_views.go, internal/tui/mcp_failed_state_test.go, internal/tui/mcp_state.go, internal/tui/mcp_view.go, internal/tui/model.go, internal/tui/options.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.