Skip to content

fix: make experimental MCP server initialization non-fatal to thv serve#4393

Merged
peppescg merged 1 commit intomainfrom
issues/4392
Mar 27, 2026
Merged

fix: make experimental MCP server initialization non-fatal to thv serve#4393
peppescg merged 1 commit intomainfrom
issues/4392

Conversation

@peppescg
Copy link
Copy Markdown
Contributor

@peppescg peppescg commented Mar 26, 2026

Summary

  • Move mcpserver.New() inside the goroutine so that a failure in MCP server creation (e.g., registry authentication error) only terminates the goroutine instead of crashing the entire thv serve process
  • The main HTTP API server continues running normally, enabling graceful degradation when the experimental MCP feature encounters issues

Fixes #4392

Test plan

  • Run thv serve --experimental-mcp with valid registry credentials → both HTTP API and MCP server start normally
  • Run thv serve --experimental-mcp with expired/invalid registry credentials → HTTP API starts normally, MCP failure is logged but process stays alive
  • Run thv serve without --experimental-mcp → no change in behavior

Move mcpserver.New() inside the goroutine so that a failure in MCP
server creation (e.g. registry authentication error) only terminates
the goroutine instead of crashing the entire thv serve process.
The main HTTP API server continues running normally.

Fixes #4392

Made-with: Cursor
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 26, 2026
@peppescg peppescg self-assigned this Mar 26, 2026
@peppescg peppescg changed the title Make experimental MCP server initialization non-fatal to thv serve fix: make experimental MCP server initialization non-fatal to thv serve Mar 26, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.52%. Comparing base (8fc2c71) to head (849b2a2).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4393      +/-   ##
==========================================
+ Coverage   69.47%   69.52%   +0.04%     
==========================================
  Files         485      485              
  Lines       49805    49805              
==========================================
+ Hits        34603    34625      +22     
+ Misses      12523    12502      -21     
+ Partials     2679     2678       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@jerm-dro jerm-dro left a comment

Choose a reason for hiding this comment

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

question: What's the use case for thv serve continuing to run when the server fails to start? Continuing without it means the process looks healthy but isn't doing what was asked.

The original crash-on-failure behavior gives immediate, unmissable feedback. Is there a scenario where the rest of the process has independent value?

@peppescg
Copy link
Copy Markdown
Contributor Author

peppescg commented Mar 26, 2026

What's the use case for thv serve continuing to run when the server fails to start?

This issue kills the thv process, including the main HTTP API server ( thv serve). The desktop app cannot do anything cause the process was killed.

This happens when a registry is not correctly authenticated, and when you restart the http server and the thv process ( for instance restarting the desktop app), the process cannot be spawned and the http server as well.
This is caused by the toolhive mcp ( we are spawning the thv http server using experimental mcp) that is doing a registry request under the hood, but if the registry is not authenticated this kills the process.

Is there a scenario where the rest of the process has independent value?

Thv serve cannot works if the thv process is not up and running, so I cannot call registry api for authenticating the registry or reset it

Copy link
Copy Markdown
Contributor

@jerm-dro jerm-dro left a comment

Choose a reason for hiding this comment

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

Thanks for explaining

@peppescg peppescg merged commit e1ade3c into main Mar 27, 2026
38 checks passed
@peppescg peppescg deleted the issues/4392 branch March 27, 2026 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Thv serve crashes when experimental MCP server fails to initialize

2 participants