perf(boot): parallelize independent boot I/O + drop redundant MCP probes (#130) - #265
Merged
Conversation
…bes (#130) Findings 1-3 from #130 (Finding 4, early ChatUI render, deferred to a browser-verified follow-up): 1. Drop the listTools() health check in MCPClient.ensureConnected() — it added a full round trip to every callTool/listPrompts/getPrompt. Trust the connected flag; callTool's reconnect-and-retry is the real safety net for the only frequent long-lived op, and boot-time prompt reads run right after a fresh connect. 2. listMcpToolsWithRetry now awaits connect() + getTools() (cache) instead of a second listTools() round trip — connect already cached the tools. 3. Fire mcp.connect() and the system-prompt fetch at the top of main(), and Promise.all the catalog walk + map-style load instead of awaiting them serially. The MCP cold-start now overlaps catalog+map+prompt I/O. Tests: mcp-client ensureConnected/onReconnect specs updated for the new no-probe contract (staleness now surfaces via callTool / explicit reconnect). Full suite green (334). main.js boot is browser-bootstrap (0% harness) — verify via the cold-load network waterfall on padus per the issue's validation plan.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
This was referenced Jun 23, 2026
cboettig
added a commit
that referenced
this pull request
Jun 29, 2026
_doConnect() set this.connected=true before populating this.tools via listTools(). Since #265 (v3.16.0) the boot retry path trusts the cache (await mcp.connect(); mcp.getTools()) instead of a fresh listTools() round trip. When the eager boot connect had flipped the flag but its listTools() hadn't resolved, listMcpToolsWithRetry's connect() short- circuited and getTools() returned []: zero remote tools registered, no exception, so the hardcoded-query fallback never fired and there is no onReconnect recovery on initial connect. The session ran local-only for its whole life — register_hex_tiles and all 5 other MCP tools missing. Proxy logs: tools_count=18 (=18 local + 0 MCP) first appears 6/28 as apps adopted v3.16/v3.17; global-30x30 today is intermittent (24 healthy / 18 degraded), the signature of a race. qwen3 sessions on padus + global-30x30 correctly reported 'register_hex_tiles isn't available'. Fix: cache tools before flipping connected, so connected===true always implies tools ready. Defense-in-depth: listMcpToolsWithRetry treats an empty list as a failure (retry + query fallback). Regression test locks the invariant that connect() never reports connected with an empty cache.
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
1 participant
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.
Addresses Findings 1–3 of #130. Finding 4 (render the welcome screen early via an
ui.setAgent()setter) is deferred to a focused follow-up — it's a browser-bound UX refactor ofChatUI.init()that needs visual verification, and I didn't want to bundle a boot-path regression risk with these safe wins.Changes
1. Drop the
listTools()health check inMCPClient.ensureConnected().It ran a full round trip before every
callTool/listPrompts/getPrompt. Now it trusts theconnectedflag;callTool's existing reconnect-and-retry branch is the real safety net for the only frequent, long-lived op, and boot-time prompt reads happen right after a fresh connect (never stale).2.
listMcpToolsWithRetryuses the connect cache.Was
await mcp.listTools()(ensureConnected probe + a second list round trip) even thoughconnect()already cached the tools. Nowawait mcp.connect(); return mcp.getTools()— dedupes with the eager boot connect, no extra round trip.3. Parallelize independent boot I/O.
mcp.connect()and thesystem-prompt.mdfetch now fire at the top ofmain(), and the STAC catalog walk + map-style load run viaPromise.allinstead of two serialawaits. The MCP cold-start overlaps catalog + map + prompt I/O (previously connect fired only after both were awaited, so it never overlapped).Net: removes per-call probe round trips and collapses several serialized boot I/Os into one overlapping wait.
Tests
test/mcp-client.test.jsupdated for the new no-probe contract:ensureConnectedon a live connection makes no client and no extralistToolscall; staleness now surfaces viacallTool/ explicitreconnect(); the twoonReconnectspecs drivereconnect()directly. Full suite green (334).Verification
app/main.jsis browser-bootstrap (0% harness). Per the issue's validation plan, verify with a cold-load network waterfall on padus/template (disable cache, throttle): confirm total time-to-ready is unchanged-or-better and the MCP connect now overlaps the catalog/map fetches. The MCP-client change is covered by unit tests.Deferred
Finding 4 (early welcome render): split
ChatUI.init()into agent-independent chrome + ansetAgent()that wires callbacks/model-selector and enables the input. Worth its own PR with jsdom tests + a browser pass, since it changes boot ordering of the UI.