Skip to content

Add MCP Connection diagnostics and CORS hint to web-ui#20753

Closed
evalstate wants to merge 3 commits into
ggml-org:masterfrom
evalstate:feat/mcp-diagnoses
Closed

Add MCP Connection diagnostics and CORS hint to web-ui#20753
evalstate wants to merge 3 commits into
ggml-org:masterfrom
evalstate:feat/mcp-diagnoses

Conversation

@evalstate

Copy link
Copy Markdown
Contributor

This adds diagnostic printouts and a "check CORS" warning for Failed to Fetch scenarios when connecting to MCP Servers. Tokens etc. are redacted.
image

@evalstate evalstate requested a review from a team as a code owner March 19, 2026 11:49

@allozaur allozaur 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.

few architectural remarks :)

To sum up, I'd suggest:

  1. Moving utility functions to src/lib/utils/

api-headers.ts - Header handling:

  • sanitizeHeaders()
  • redactHeaderValue()

request-helpers.ts - Request utilities:

  • getRequestUrl()
  • getRequestMethod()
  • getRequestBody()
  • summarizeRequestBody()
  • formatDiagnosticErrorMessage()
  • extractJsonRpcMethods()
  • summarizeError()
  • getBrowserContext()
  • getConnectionHints()

constants/ - Redacted headers list:

  • REDACTED_HEADERS
  1. Splitting tests into two files

tests/unit/utils/request-helpers.test.ts - Tests for pure utility functions
tests/unit/mcp-service.test.ts - Tests for MCPService-specific logic only

Comment thread tools/server/webui/src/lib/services/mcp.service.ts
Comment thread tools/server/webui/src/lib/services/mcp.service.ts
Comment thread tools/server/webui/src/lib/services/mcp.service.ts
Comment thread tools/server/webui/src/lib/services/mcp.service.ts

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.

depending on the final shape of the code inside and outside of MCP Service class, some of these tests might need moving to separate file(s)

@allozaur

Copy link
Copy Markdown
Contributor

Superseded by #21803

@allozaur allozaur closed this Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants