Skip to content

feat(code): add MCP servers entry to main sidebar under Skills#2010

Merged
jonathanlab merged 2 commits intomainfrom
posthog-code/mcp-servers-sidebar-item
May 4, 2026
Merged

feat(code): add MCP servers entry to main sidebar under Skills#2010
jonathanlab merged 2 commits intomainfrom
posthog-code/mcp-servers-sidebar-item

Conversation

@jonathanlab
Copy link
Copy Markdown
Contributor

@jonathanlab jonathanlab commented May 4, 2026

fairly self explanatory

Adds a top-level "MCP servers" sidebar item directly underneath the
Skills item. Clicking it opens the existing MCP servers management UI
as a main view (header set via useSetHeaderContent), reusing the
McpServersSettings component already shown in Settings.

Generated-By: PostHog Code
Task-Id: 3516a98e-d297-4c74-a78e-ed7a96855927
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/code/src/renderer/features/mcp-servers/components/McpServersView.tsx:24-28
**Missing flex container causes `McpServersSettings` layout to collapse**

`McpServersSettings` renders its root element as `<Flex className="min-h-0 w-full flex-1 overflow-hidden">`. The `flex-1` only works when the parent is a flex container — in the Settings dialog this is guaranteed by `fullwidth: true`, which wraps the component in `<div className="... flex h-full min-h-0 w-full">`. Here the parent is a plain `Box` (a `div`), so `flex-1` has no effect and the component falls back to auto/content height. The internal rail and scroll area both use `min-h-0 flex-1` and will not fill the view correctly, likely cutting off content or breaking scrolling.

```suggestion
    <Flex height="100%" className="overflow-hidden min-h-0">
      <McpServersSettings />
    </Flex>
```

Reviews (1): Last reviewed commit: "feat(code): add MCP servers entry to mai..." | Re-trigger Greptile

Comment on lines +24 to +28

return (
<Box height="100%" className="overflow-hidden">
<McpServersSettings />
</Box>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing flex container causes McpServersSettings layout to collapse

McpServersSettings renders its root element as <Flex className="min-h-0 w-full flex-1 overflow-hidden">. The flex-1 only works when the parent is a flex container — in the Settings dialog this is guaranteed by fullwidth: true, which wraps the component in <div className="... flex h-full min-h-0 w-full">. Here the parent is a plain Box (a div), so flex-1 has no effect and the component falls back to auto/content height. The internal rail and scroll area both use min-h-0 flex-1 and will not fill the view correctly, likely cutting off content or breaking scrolling.

Suggested change
return (
<Box height="100%" className="overflow-hidden">
<McpServersSettings />
</Box>
<Flex height="100%" className="overflow-hidden min-h-0">
<McpServersSettings />
</Flex>
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/mcp-servers/components/McpServersView.tsx
Line: 24-28

Comment:
**Missing flex container causes `McpServersSettings` layout to collapse**

`McpServersSettings` renders its root element as `<Flex className="min-h-0 w-full flex-1 overflow-hidden">`. The `flex-1` only works when the parent is a flex container — in the Settings dialog this is guaranteed by `fullwidth: true`, which wraps the component in `<div className="... flex h-full min-h-0 w-full">`. Here the parent is a plain `Box` (a `div`), so `flex-1` has no effect and the component falls back to auto/content height. The internal rail and scroll area both use `min-h-0 flex-1` and will not fill the view correctly, likely cutting off content or breaking scrolling.

```suggestion
    <Flex height="100%" className="overflow-hidden min-h-0">
      <McpServersSettings />
    </Flex>
```

How can I resolve this? If you propose a fix, please make it concise.

- Move McpServersSettings + supporting components/hooks out of
  features/settings into a dedicated features/mcp-servers feature.
- Rename the entrypoint to McpServersView; render it as a real
  top-level view (Flex height="100%") so the installed-rail extends
  the full content height instead of collapsing to its inner content.
- Remove the MCP servers entry from the Settings sidebar (id, title,
  component map, SettingsCategory union) and drop the now-unused
  fullwidth branch in SettingsDialog.

Generated-By: PostHog Code
Task-Id: 3516a98e-d297-4c74-a78e-ed7a96855927
@jonathanlab jonathanlab requested a review from cvolzer3 May 4, 2026 16:37
@jonathanlab jonathanlab merged commit 1059ff4 into main May 4, 2026
15 checks passed
@jonathanlab jonathanlab deleted the posthog-code/mcp-servers-sidebar-item branch May 4, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants