fix: remote skill install on expired token + MCP unreachable empty response#128
Merged
Merged
Conversation
…erver against empty responses Fixes #127 — remote skills whose URL is on a known git host (e.g. raw.githubusercontent.com) could not be installed when the user had a stored-but-expired GitHub token. AuthenticatedFetch now returns null from ensureFreshIntegration instead of throwing, so public URLs proceed unauthenticated; private URLs still surface a 401/403 which the existing handler converts into the friendly re-auth prompt. Fixes #126 — the local capa server could return net::ERR_EMPTY_RESPONSE when an MCP server was unreachable: - Added a top-level try/catch in handleRequest so no request can ever close the connection without sending an HTTP response. - Wrapped handleGetOAuth2Servers (whole handler + per-server OAuth detection loop) in try/catch, mirroring the pattern used by the configure endpoint. - Added OAUTH_DETECT_TIMEOUT_MS (10 s) to all outbound fetches in OAuth2Manager.detectOAuth2Requirement so a hung server fails fast. - Added MCP_CONNECT_TIMEOUT_MS (15 s) race in MCPProxy.createHttpClient and createStdioClient so a non-responsive MCP server does not stall indefinitely. Co-authored-by: Cursor <cursoragent@cursor.com>
… fetch Co-authored-by: Cursor <cursoragent@cursor.com>
…osed errors Follow-up to the #126 work, addressing two remaining symptoms when an MCP server is unreachable (e.g. VPN dropped): 1. Stray "The socket connection was closed unexpectedly" printed during install even though it succeeded. The connect-timeout added previously used a Promise.race that left the losing promise dangling: when the socket closed after we'd timed out (or the timer fired after a successful connect), the rejection surfaced as an unhandled rejection. Replaced it with connectWithTimeout(), which attaches a no-op catch to the connect promise and clears the timer in finally. Added process-level unhandledRejection/uncaughtException handlers as a safety net for the MCP SDK transport's background sockets. 2. The /tools endpoint returned 200 { tools: [] } for unreachable servers, so the UI could not tell "no tools" apart from "unreachable". listServerTools now accepts options and handleGetServerTools calls it with throwOnError, returning 502 with a clear message ("Server unreachable: ..."; the OAuth-disconnected message is passed through untouched). The web UI tools panel now renders that error, and useServerTools no longer retries so the error shows immediately. Co-authored-by: Cursor <cursoragent@cursor.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… rejections Half-open MCP servers (accept the TCP connection but never complete the handshake, e.g. return an empty response) made client.connect() hang until our timeout fired. Two edge cases could still leak a stray "MCP connect timed out after 15000ms" unhandled rejection out to the process/UI: - If client.connect() threw synchronously, the timeout timer was never cleared and rejected ~15s later into the void. - On timeout we left the half-open client/transport open, so its socket could emit further late errors. connectWithTimeout now: builds the timer only after connect starts, always clears it in finally (covering synchronous throws), best-effort closes the client + transport on timeout, and keeps the no-op catch on the connect promise so a late socket-close rejection stays handled. The timeout is now parameterized so it can be unit-tested with a short window. Adds tests asserting the timeout path tears down the connection and never produces an unhandled rejection across the timeout, late-rejection, success, and synchronous-throw cases. Co-authored-by: Cursor <cursoragent@cursor.com>
… text CodeQL (js/stack-trace-exposure) flagged the /tools handler returning raw error.message to the client, which can leak stack-trace-derived detail. Both handleGetServerTools and the handleGetOAuth2Servers catch now log the full detail server-side but return controlled, generic messages: tools failures report "Server unreachable: \"<id>\" could not be contacted" (or an auth-required prompt for OAuth-disconnected servers), and the oauth-servers handler returns a generic "Failed to load OAuth2 servers". The UI still shows a clear, actionable error without exposing internal exception text. Co-authored-by: Cursor <cursoragent@cursor.com>
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
2 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.
Summary
Fixes [Bug] Cannot install a skill from a remote URL #127 — Installing a
type: remoteskill from a public URL (e.g.raw.githubusercontent.com) failed with "Git integration token has expired" when the user had a stored-but-expired GitHub token.AuthenticatedFetch.ensureFreshIntegrationnow returnsnullinstead of throwing when the token cannot be refreshed, so the request proceeds unauthenticated. Genuinely private repos still get a 401/403, which the existing handler already converts into a friendly re-auth prompt.Fixes [Bug] When MCP server is unreachable, the local capa server returns an empty response (error) #126 — The local capa server could return
net::ERR_EMPTY_RESPONSE(empty TCP response) when an MCP server was unreachable. Three layers of protection were added:handleRequest— no request can ever close the connection without an HTTP response.handleGetOAuth2Serverswrapped in try/catch (whole handler + per-server detection loop), mirroring the existing pattern in the configure endpoint.AbortSignal.timeouton OAuth detection fetches inOAuth2Manager, and a 15 sPromise.raceonclient.connect()inMCPProxy, so a hung/unreachable server fails fast rather than stalling indefinitely.Test plan
AuthenticatedFetchtests: expired-token case now asserts unauthenticated fetch proceeds (not a throw); refresh-failure-classification tests updated accordingly.OAuth2Manager.detectOAuth2Requirementtests: returnsnullon AbortError (unreachable server) and on non-401 response.