Skip to content

fix(typecheck): add MCP component view types - #1564

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:codex/typecheck-mcp-component-types
Jun 10, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
chioarub:codex/typecheck-mcp-component-types

Conversation

@chioarub

@chioarub chioarub commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Refs #1486

Summary

  • add the missing shared MCP component types for server list/menu/detail view state
  • type MCP settings/list/tool component entry points so existing React compiler caches do not collapse state and props to unknown/never[]
  • annotate MCP menu option arrays with the existing OptionWithDescription type

Why

The /mcp UI imports ./types.js, but the source file was missing. That creates direct TS2307 failures and masks follow-on never[] and unknown inference errors in the MCP list, server menus, and tool views.

Duplicate check

I checked open PR file lists before opening this. The only overlap is broad feature PR #1257, which also adds an MCP types file while changing many unrelated areas. This PR is the focused #1486 cleanup: it adds the shared MCP component types and fixes the local menu/view-state inference errors without the unrelated feature changes.

Validation

  • bun run typecheck still reports unrelated pre-existing diagnostics, but this branch reduces diagnostic lines from 1043 to 1012.
  • Focused MCP component/type diagnostics went from 31 to 0.
  • Remaining nearby diagnostics are unrelated plugin command type files and existing ElicitationDialog hook arity errors.

Summary by CodeRabbit

  • Refactor
    • Enhanced internal code structure and type safety for MCP components to improve maintainability and reduce potential runtime errors.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e8382534-25aa-4957-82c2-0f66a2658410

📥 Commits

Reviewing files that changed from the base of the PR and between 9e942da and fde9e51.

📒 Files selected for processing (8)
  • src/components/mcp/MCPAgentServerMenu.tsx
  • src/components/mcp/MCPListPanel.tsx
  • src/components/mcp/MCPRemoteServerMenu.tsx
  • src/components/mcp/MCPSettings.tsx
  • src/components/mcp/MCPStdioServerMenu.tsx
  • src/components/mcp/MCPToolDetailView.tsx
  • src/components/mcp/MCPToolListView.tsx
  • src/components/mcp/types.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: smoke-and-tests
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise

Files:

  • src/components/mcp/types.ts
  • src/components/mcp/MCPToolListView.tsx
  • src/components/mcp/MCPAgentServerMenu.tsx
  • src/components/mcp/MCPToolDetailView.tsx
  • src/components/mcp/MCPStdioServerMenu.tsx
  • src/components/mcp/MCPListPanel.tsx
  • src/components/mcp/MCPRemoteServerMenu.tsx
  • src/components/mcp/MCPSettings.tsx
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/components/mcp/types.ts
  • src/components/mcp/MCPToolListView.tsx
  • src/components/mcp/MCPAgentServerMenu.tsx
  • src/components/mcp/MCPToolDetailView.tsx
  • src/components/mcp/MCPStdioServerMenu.tsx
  • src/components/mcp/MCPListPanel.tsx
  • src/components/mcp/MCPRemoteServerMenu.tsx
  • src/components/mcp/MCPSettings.tsx
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

  • Search existing issues and discussions before opening a new thread.
  • Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
  • Use issues for confirmed bugs and actionable feature work.
  • Use discussions for setup help, ideas, and general community conversation.
  • For larger changes, open an issue first so the scope is clear before implementation.
  • For security reports, follow SECURITY.md.

Pull Requests

Every PR needs a reason. Your PR description must include:

  • what changed and why
  • the user or developer impact
  • the exact checks you ran
  • a linked issue when one exists, using Fixes fix: skip assertMinVersion for third-party providers #123, `Closes `#123, or another clear link
  • screenshots when the PR touches UI, terminal presentation, or the VS Code extension
  • which provider path was tested when the PR changes provider behavior

The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.

Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.

What Gets Closed Without Review

PRs may be closed without review...

Files:

  • src/components/mcp/types.ts
  • src/components/mcp/MCPToolListView.tsx
  • src/components/mcp/MCPAgentServerMenu.tsx
  • src/components/mcp/MCPToolDetailView.tsx
  • src/components/mcp/MCPStdioServerMenu.tsx
  • src/components/mcp/MCPListPanel.tsx
  • src/components/mcp/MCPRemoteServerMenu.tsx
  • src/components/mcp/MCPSettings.tsx
🔇 Additional comments (15)
src/components/mcp/MCPAgentServerMenu.tsx (1)

9-9: LGTM!

Also applies to: 105-105

src/components/mcp/MCPRemoteServerMenu.tsx (1)

23-23: LGTM!

Also applies to: 470-470

src/components/mcp/MCPStdioServerMenu.tsx (1)

13-13: LGTM!

Also applies to: 59-59

src/components/mcp/types.ts (1)

1-77: LGTM!

src/components/mcp/MCPSettings.tsx (5)

21-21: LGTM!


38-38: LGTM!


46-46: LGTM!


71-76: LGTM!


101-101: Normalization ensures isAuthenticated is always boolean at runtime.

The ?? false operator converts undefined to false, so the isAuthenticated field will always be a boolean value even though the type allows isAuthenticated?: boolean. This is a minor behavior change from allowing undefined to be stored. The change makes the runtime data more predictable and is consistent across both SSE and HTTP paths.

Also applies to: 109-109

src/components/mcp/MCPListPanel.tsx (3)

92-92: LGTM!


109-109: LGTM!


486-488: LGTM!

src/components/mcp/MCPToolListView.tsx (2)

20-20: LGTM!


64-64: LGTM!

src/components/mcp/MCPToolDetailView.tsx (1)

14-14: LGTM!


📝 Walkthrough

Walkthrough

This PR adds comprehensive TypeScript typing to MCP components. A new types.ts module introduces ServerInfo union types and UI state definitions, then component signatures, menu options, and internal state across five MCP components are updated to use explicit type annotations.

Changes

MCP Component Typing

Layer / File(s) Summary
MCP Type Definitions
src/components/mcp/types.ts
New types module exports server info variants (StdioServerInfo, SSEServerInfo, HTTPServerInfo, ClaudeAIServerInfo), unified ServerInfo union, AgentMcpServerInfo, and MCPViewState discriminated union for UI states.
Component Signature Typing
src/components/mcp/MCPListPanel.tsx, src/components/mcp/MCPSettings.tsx, src/components/mcp/MCPToolDetailView.tsx, src/components/mcp/MCPToolListView.tsx
Component exports now explicitly type Props parameters and return types (MCPSettings and MCPToolDetailView declare React.ReactNode returns).
Select Menu Options Typing
src/components/mcp/MCPAgentServerMenu.tsx, src/components/mcp/MCPRemoteServerMenu.tsx, src/components/mcp/MCPStdioServerMenu.tsx
Three menu components import OptionWithDescription and consistently type menuOptions arrays as OptionWithDescription[].
State and Implementation Typing
src/components/mcp/MCPSettings.tsx, src/components/mcp/MCPListPanel.tsx
MCPSettings state hooks typed (viewState: MCPViewState, servers: ServerInfo[]), async server-building code types serverInfos and normalizes isAuthenticated to boolean via ?? false; MCPListPanel casts agentServers to AgentMcpServerInfo[] and types helper parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes


Suggested reviewers

  • jatmn
  • kevincodex1
🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed ⚠️ Warning PR touches MCP but doesn't explicitly call out risk surface. Touches auth code (isAuthenticated ?? false, ClaudeAuthProvider) requiring documented risk analysis. Add risk assessment to PR: clarify whether isAuthenticated default-to-false blocks, confirm no auth provider behavior/MCP execution changes, document impact.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the main change: adding MCP component view types for TypeScript type checking, matching the diff scope.
Description check ✅ Passed Description covers Summary (what/why changed), provides context (TS2307 errors, diagnostic reduction), mentions duplicate checking, and includes validation results from typecheck.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Hidden Policy Change ✅ Passed PR adds only TypeScript type definitions and annotations. No changes to product, trust model, routing, telemetry, or permissions detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution. I do not see any actionable issues from my review.

@kevincodex1 LGTM

@kevincodex1
kevincodex1 merged commit 548bffc into Twigpine:main Jun 10, 2026
3 checks passed
@chioarub
chioarub deleted the codex/typecheck-mcp-component-types branch June 10, 2026 05:58
deagwon97 pushed a commit to deagwon97/openclaude that referenced this pull request Jun 11, 2026
hotmanxp pushed a commit to hotmanxp/openclaude that referenced this pull request Jun 12, 2026
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.

3 participants