fix(runtime): bound per-toolset tool listing during startup (#3137)#3154
Merged
Conversation
emitToolsProgressively called toolset.Tools(ctx) with no timeout, so a
toolset whose Tools() blocks forever (for example a wedged MCP stdio
subprocess that never answers tools/list) prevented the loop from ever
sending its terminal ToolsetInfo{Loading:false}. The sidebar stayed on
"Loading tools..." forever and EmitStartupInfo's goroutine leaked.
Add listToolsWithTimeout: it runs Tools() in a goroutine and selects on a
per-toolset deadline, so even a toolset that ignores context cancellation
cannot stall startup. A timed-out toolset is skipped with a warning and the
terminal ToolsetInfo is still sent, so the sidebar resolves and shows the
tools that loaded. Timeout defaults to 10s, configurable via
WithToolListTimeout.
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
dgageot
approved these changes
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3137.
what
emitToolsProgressivelycalledtoolset.Tools(ctx)with no timeout, so if a toolset'sTools()blocked forever (e.g. a wedged MCP stdio server that never answerstools/list), startup tool loading never finished. The terminalToolsetInfo{Loading:false}was never sent, so the sidebar stayed on "Loading tools..." andEmitStartupInfo's goroutine leaked.This adds
listToolsWithTimeout, which runsTools()in a goroutine and bounds it with a per-toolset deadline (default 10s, configurable viaWithToolListTimeout). I went with goroutine + select instead of a plain context timeout because the MCP client detaches ctx withWithoutCancel, so a wedged server might not honor cancellation. A timed-out toolset is skipped with a warning, and the terminalToolsetInfois always sent so the sidebar resolves and shows whatever loaded fine. The toolset is not torn down, so a slow-but-responsive one is listed again on the next turn.how it maps to the issue
the issue lists three expected behaviours:
EmitStartupInforeturns. i left the 30sstopToolSetsbound alone since the issue lists it as "consider" and lowering it risks cutting off a legit graceful shutdown. happy to do it as a follow-up if you'd prefer.possible follow-up (not in this PR)
a server that wedges during
initialize/Start()would stall startup the same way, and that path is also unbounded (Connectdetaches ctx, the supervisor has no connect deadline). the same pattern would fix it. kept it out here to stay close to the issue's stated root cause (Tools()).test
added
TestEmitStartupInfo_SkipsToolsetWhoseListingHangs(passes under-race). also reproduced by hand: hangs before, resolves after, using a hung toolset plus a tiny MCP stdio server that finishes initialize then never answerstools/list.