Skip to content

feat: support remote MCP servers via Streamable HTTP - #14

Open
keybrdist wants to merge 2 commits into
mainfrom
feat/remote-mcp-support
Open

feat: support remote MCP servers via Streamable HTTP#14
keybrdist wants to merge 2 commits into
mainfrom
feat/remote-mcp-support

Conversation

@keybrdist

Copy link
Copy Markdown
Owner

Ports the remote MCP server support from #8 by @orionpax1997 onto current main, without that PR's packaging/rebrand changes (which had conflicted with #12/#13).

What's included

  • McpServerConfig is now a discriminated union: LocalMcpServerConfig | RemoteMcpServerConfig. type: "remote" + url selects the SDK's StreamableHTTPClientTransport; local stdio behavior is unchanged and remains the default.
  • Optional headers on remote configs (with the existing ${VAR} env expansion, since the whole config is expanded before connect) for e.g. bearer auth.
  • skill-loader accepts remote entries in direct-format mcp.json files.
  • Remote-specific connection error messages (URL, reachability/auth hints).
  • Addresses the Copilot review note on feat: support remote mcp #8: normalizeCommand/normalizeEnv keep the McpServerConfig union signature and fail fast on remote configs instead of narrowing their parameter type.
  • New/updated types re-exported from src/index.ts; README documents the remote format.

Not included from #8

Testing

  • npm run build clean, npm test 30/30 passing.
  • New tests: remote config discovery from frontmatter, direct-format mcp.json remote entries, rejection of configs with neither command nor type: remote, and remote rejection in normalizeCommand/normalizeEnv.

Closes #8

Co-authored-by: orionpax1997

Skills can now declare remote MCP servers alongside local stdio ones:

  mcp:
    my-server:
      type: remote
      url: https://mcp.example.com/mcp
      headers:
        Authorization: Bearer ${API_TOKEN}

- Split McpServerConfig into a LocalMcpServerConfig | RemoteMcpServerConfig
  discriminated union (type: "remote" selects StreamableHTTPClientTransport)
- Accept remote configs in direct-format mcp.json files
- normalizeCommand/normalizeEnv keep the union signature and fail fast on
  remote configs
- Document remote config in README; add loader and normalize tests

Ported from #8 by @orionpax1997, minus that PR's packaging changes.

Co-authored-by: orionpax1997 <orionpax.1997@outlook.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 04:28

Copilot AI left a comment

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.

Pull request overview

Adds first-class support for remote MCP servers (in addition to existing local/stdio servers) by introducing a discriminated McpServerConfig union and wiring the MCP manager to use the SDK’s Streamable HTTP transport when type: "remote" is configured. This keeps existing local behavior as the default while enabling URL-based remote connections with optional request headers.

Changes:

  • Introduce LocalMcpServerConfig | RemoteMcpServerConfig and re-export the new types for external consumers.
  • Add remote connection support in skill-mcp-manager using StreamableHTTPClientTransport, including remote-specific connection failure messaging.
  • Extend skill-loader (and tests) to recognize remote entries in direct-format mcp.json, and update docs/tests accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/utils/env-vars.ts Fail-fast when normalizeCommand / normalizeEnv are accidentally used with remote configs.
src/types.ts Define LocalMcpServerConfig, RemoteMcpServerConfig, and the McpServerConfig discriminated union.
src/skill-mcp-manager.ts Select stdio vs Streamable HTTP transport based on config; add remote connect error messaging.
src/skill-loader.ts Recognize remote entries in direct-format mcp.json configs.
src/index.ts Re-export new config types for downstream usage.
src/tests/skill-loader.test.ts Add coverage for remote configs via frontmatter and direct-format mcp.json.
src/tests/normalize-command.test.ts Add coverage for rejecting remote configs in normalizeCommand/normalizeEnv.
README.md Document the remote MCP server configuration format and example.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/skill-mcp-manager.ts
Comment on lines +135 to +143
let url: URL
try {
url = new URL(config.url)
} catch {
throw new Error(
`MCP server "${info.serverName}" has an invalid URL: ${config.url}\n\n` +
`The URL must be a valid HTTP or HTTPS URL.`
)
}
@keybrdist

Copy link
Copy Markdown
Owner Author

@copilot address all merge conflicts, build failures, and review comments on this PR. When you have addressed a code review issue mark it as resolved.

…oteTransport

Co-authored-by: keybrdist <3534206+keybrdist@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 8, 2026 04:33

Copilot AI commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

@copilot address all merge conflicts, build failures, and review comments on this PR. When you have addressed a code review issue mark it as resolved.

Addressed the createRemoteTransport review note in c1eabfd:

  • Added an explicit check for a missing url field that throws a clear error before new URL() is ever called.
  • Added a scheme validation (url.protocol !== 'http:' && url.protocol !== 'https:') so non-HTTP schemes like ftp: are rejected immediately with a descriptive message.

All 30 tests still pass.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/skill-mcp-manager.ts:103

  • isRemoteConfig only checks config.type === "remote". If a user misspells the discriminator (e.g. type: "Remote") or provides some other invalid value via JSON/frontmatter, the code will treat it as a local config and emit a misleading "missing command" error. Consider validating type explicitly and failing fast with a clearer message.
  const isRemoteConfig = (config: McpServerConfig): config is RemoteMcpServerConfig => {
    return config.type === 'remote'
  }

README.md:166

  • The README introduces LocalMcpServerConfig and RemoteMcpServerConfig, but it no longer shows how these relate back to the McpServerConfig type that the plugin/API surface uses. Adding the union alias here would make the documented shape match the exported types and reduce confusion for consumers.
}

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