Skip to content

fix: handle invalid mcp urls#25019

Merged
Hona merged 1 commit into
anomalyco:devfrom
Hona:fix/mcp-invalid-url
Apr 29, 2026
Merged

fix: handle invalid mcp urls#25019
Hona merged 1 commit into
anomalyco:devfrom
Hona:fix/mcp-invalid-url

Conversation

@Hona
Copy link
Copy Markdown
Member

@Hona Hona commented Apr 29, 2026

Summary

  • validate remote MCP URLs before constructing transports
  • report invalid MCP URLs as failed MCP status instead of crashing startup/auth

Verification

  • bun typecheck from packages/opencode
  • bad Context7 MCP config now reports Invalid MCP URL for "context7"
  • valid Context7 MCP URL still connects

Copilot AI review requested due to automatic review settings April 29, 2026 23:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds validation for remote MCP server URLs so invalid configuration no longer throws during MCP startup or OAuth auth flows, and instead surfaces a clean “failed” MCP status/error.

Changes:

  • Introduces a shared remoteURL() helper that validates/parses remote MCP URLs before transport construction.
  • Updates remote MCP connection setup to return a failed status when the URL is invalid (instead of throwing on new URL(...)).
  • Updates OAuth start/auth transport creation to use the validated URL (and throws a clear “Invalid MCP URL…” error for auth flows).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Hona Hona merged commit ea89925 into anomalyco:dev Apr 29, 2026
11 checks passed
Bojun-Vvibe added a commit to Bojun-Vvibe/oss-contributions that referenced this pull request Apr 30, 2026
- anomalyco/opencode#25062 (merge-as-is) — defensive optional-chain on legacy session restore

- anomalyco/opencode#25019 (merge-as-is) — per-server fail-soft on invalid MCP url

- anomalyco/opencode#25032 (merge-as-is) — middleware-pair test + directory AGENTS.md

- openai/codex#20278 (merge-after-nits) — workspace plugin sharing v2 RPCs
teknium1 added a commit to NousResearch/hermes-agent that referenced this pull request May 16, 2026
Port from anomalyco/opencode#25019 ("fix: handle invalid mcp urls").

Previously: a typo in `config.yaml` (missing scheme, wrong scheme,
empty string, non-string value) slipped past `_is_http()` and hit
`httpx.URL(url)` or `streamablehttp_client(url, ...)` deep in the
transport layer. That raised a generic exception which went through
the reconnect-backoff loop, so a bad URL caused _MAX_INITIAL_CONNECT_RETRIES
attempts with doubling backoff — about a minute of pointless retries
plus an opaque error — before the server was marked failed.

Now: we validate the URL once, at the top of `run()`, before
entering the retry loop. A malformed URL raises `InvalidMcpUrlError`
(a `ValueError` subclass) with a message that names the offending
server and explains exactly what was wrong. `_ready` is set and
`_error` is populated, so `start()` re-raises and the server shows
up as failed in `hermes mcp list` without any backoff burn.

Validation rules:
- Must be a string (rejects None, dict, int)
- Must be non-empty (rejects '' and whitespace-only)
- Scheme must be http or https (rejects file://, ws://, stdio://)
- Must have a non-empty host (rejects http:///, http://:8080)

Tests (21 new cases in tests/tools/test_mcp_invalid_url.py):
- TestValidUrlsAccepted: http, https, IPv6, ports, paths, query strings
- TestInvalidUrlsRejected: every rejection path above + clear error text
- TestErrorIsValueError: downstream code catching ValueError still works

E2E verified: a misconfigured server with `url: not-a-valid-url`
now fails in <0.001s with the clear error, instead of minutes of retries.

Doesn't touch stdio servers (they use `command`, not `url`) — the
validator only fires when `_is_http()` returns True.
DIZ-admin pushed a commit to DIZ-admin/hermes-agent that referenced this pull request May 16, 2026
…rch#27105)

Port from anomalyco/opencode#25019 ("fix: handle invalid mcp urls").

Previously: a typo in `config.yaml` (missing scheme, wrong scheme,
empty string, non-string value) slipped past `_is_http()` and hit
`httpx.URL(url)` or `streamablehttp_client(url, ...)` deep in the
transport layer. That raised a generic exception which went through
the reconnect-backoff loop, so a bad URL caused _MAX_INITIAL_CONNECT_RETRIES
attempts with doubling backoff — about a minute of pointless retries
plus an opaque error — before the server was marked failed.

Now: we validate the URL once, at the top of `run()`, before
entering the retry loop. A malformed URL raises `InvalidMcpUrlError`
(a `ValueError` subclass) with a message that names the offending
server and explains exactly what was wrong. `_ready` is set and
`_error` is populated, so `start()` re-raises and the server shows
up as failed in `hermes mcp list` without any backoff burn.

Validation rules:
- Must be a string (rejects None, dict, int)
- Must be non-empty (rejects '' and whitespace-only)
- Scheme must be http or https (rejects file://, ws://, stdio://)
- Must have a non-empty host (rejects http:///, http://:8080)

Tests (21 new cases in tests/tools/test_mcp_invalid_url.py):
- TestValidUrlsAccepted: http, https, IPv6, ports, paths, query strings
- TestInvalidUrlsRejected: every rejection path above + clear error text
- TestErrorIsValueError: downstream code catching ValueError still works

E2E verified: a misconfigured server with `url: not-a-valid-url`
now fails in <0.001s with the clear error, instead of minutes of retries.

Doesn't touch stdio servers (they use `command`, not `url`) — the
validator only fires when `_is_http()` returns True.
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