fix: remote skill install on expired token + MCP unreachable empty response

src/server/__tests__/mcp-handler.integration.test.ts

@@ -610,3 +610,55 @@ describe('getAllShellTools / getShellToolSchema (lazy MCP schema)', () => {
     expect(schema.inputSchema.properties.pattern).toBeDefined();
   });
 });
+
+// ─── listServerTools: surface unreachable servers (issue #126) ───────────────
+//
+// The /tools API handler relies on `listServerTools` forwarding
+// `throwOnError` so an unreachable MCP server produces an HTTP error the UI can
+// render ("Server unreachable") instead of a silent empty list that looks like
+// "this server has no tools".
+
+describe('listServerTools (throwOnError pass-through)', () => {
+  let h: Harness;
+
+  const caps: Capabilities = {
+    providers: ['claude-code'],
+    options: {},
+    skills: [],
+    servers: [{ id: 'brave', def: { url: 'http://127.0.0.1:1/mcp' } } as any],
+    tools: [],
+  };
+
+  beforeEach(() => {
+    h = makeHarness(caps);
+  });
+
+  afterEach(() => destroyHarness(h));
+
+  it('forwards throwOnError to the proxy', async () => {
+    let seenOptions: any;
+    (h.mcp as any).mcpProxy.listTools = async (_id: string, _def: unknown, options: unknown) => {
+      seenOptions = options;
+      return [];
+    };
+
+    await h.mcp.listServerTools('brave', caps, { throwOnError: true });
+    expect(seenOptions).toEqual({ throwOnError: true });
+  });
+
+  it('propagates connection failures instead of swallowing them', async () => {
+    (h.mcp as any).mcpProxy.listTools = async () => {
+      throw new Error('Could not connect to MCP server "brave"');
+    };
+
+    await expect(
+      h.mcp.listServerTools('brave', caps, { throwOnError: true }),
+    ).rejects.toThrow(/Could not connect/);
+  });
+
+  it('returns an empty list for a reachable server with no tools', async () => {
+    (h.mcp as any).mcpProxy.listTools = async () => [];
+    const tools = await h.mcp.listServerTools('brave', caps, { throwOnError: true });
+    expect(tools).toEqual([]);
+  });
+});

src/server/__tests__/mcp-proxy.test.ts

@@ -78,6 +78,92 @@ describe('mcp-proxy', () => {
     });
   });
 
+  describe('connectWithTimeout', () => {
+    // Capture unhandled rejections during each test so we can assert the
+    // timeout path never leaks one (the bug behind the stray
+    // "MCP connect timed out after 15000ms" the user saw in the UI/logs).
+    let unhandled: unknown[];
+    const onUnhandled = (reason: unknown) => unhandled.push(reason);
+
+    beforeEach(() => {
+      unhandled = [];
+      process.on('unhandledRejection', onUnhandled);
+    });
+
+    afterEach(() => {
+      process.off('unhandledRejection', onUnhandled);
+    });
+
+    function makeProxy(): any {
+      return new MCPProxy(makeMockDb(), 'proj-1', '/tmp/project');
+    }
+
+    it('rejects with a timeout error and tears down a hung connect', async () => {
+      const proxy = makeProxy();
+      let closed = false;
+      const client = {
+        connect: () => new Promise<void>(() => {}), // never resolves
+        close: async () => {
+          closed = true;
+        },
+      };
+      const transport = { close: async () => {} };
+
+      await expect(proxy.connectWithTimeout(client, transport, 30)).rejects.toThrow(/timed out/);
+      expect(closed).toBe(true);
+    });
+
+    it('does not leak an unhandled rejection when the connect rejects after the timeout', async () => {
+      const proxy = makeProxy();
+      let rejectConnect: (e: unknown) => void = () => {};
+      const client = {
+        connect: () =>
+          new Promise<void>((_, reject) => {
+            rejectConnect = reject;
+          }),
+        close: async () => {},
+      };
+      const transport = { close: async () => {} };
+
+      await expect(proxy.connectWithTimeout(client, transport, 20)).rejects.toThrow(/timed out/);
+
+      // Simulate the real socket closing *after* we already gave up.
+      rejectConnect(new Error('The socket connection was closed unexpectedly'));
+      await new Promise((r) => setTimeout(r, 20));
+
+      expect(unhandled).toHaveLength(0);
+    });
+
+    it('resolves on a successful connect and clears the timer (no late rejection)', async () => {
+      const proxy = makeProxy();
+      const client = { connect: async () => {}, close: async () => {} };
+      const transport = { close: async () => {} };
+
+      await expect(proxy.connectWithTimeout(client, transport, 50)).resolves.toBeUndefined();
+
+      // Wait well past the timeout window to prove the timer was cleared.
+      await new Promise((r) => setTimeout(r, 80));
+      expect(unhandled).toHaveLength(0);
+    });
+
+    it('propagates a synchronous connect throw without leaking a timer', async () => {
+      const proxy = makeProxy();
+      const client = {
+        connect: () => {
+          throw new Error('boom');
+        },
+        close: async () => {},
+      };
+      const transport = { close: async () => {} };
+
+      await expect(proxy.connectWithTimeout(client, transport, 30)).rejects.toThrow(/boom/);
+
+      // If the timer had been scheduled before the throw, it would fire here.
+      await new Promise((r) => setTimeout(r, 50));
+      expect(unhandled).toHaveLength(0);
+    });
+  });
+
   describe('OAuth2 disconnected handling', () => {
     function makeOauthServerDef(): MCPServerDefinition {
       return {

src/server/__tests__/oauth-manager.test.ts

@@ -63,6 +63,34 @@ describe('OAuth2Manager', () => {
     });
   });
 
+  describe('detectOAuth2Requirement', () => {
+    const originalFetch = globalThis.fetch;
+
+    afterEach(() => {
+      globalThis.fetch = originalFetch;
+    });
+
+    it('returns null (does not throw) when the server is unreachable', async () => {
+      // Simulate a connection-refused / aborted fetch — the same class of error
+      // that an unreachable MCP server produces at the network layer.
+      globalThis.fetch = (async () => {
+        throw new DOMException('The operation was aborted.', 'AbortError');
+      }) as unknown as typeof fetch;
+
+      const manager = new OAuth2Manager(makeMockDb());
+      const result = await manager.detectOAuth2Requirement('http://192.0.2.1:9999/mcp');
+      expect(result).toBeNull();
+    });
+
+    it('returns null when the MCP server returns a non-401 status', async () => {
+      globalThis.fetch = (async () => new Response('', { status: 200 })) as unknown as typeof fetch;
+
+      const manager = new OAuth2Manager(makeMockDb());
+      const result = await manager.detectOAuth2Requirement('http://localhost:9999/mcp');
+      expect(result).toBeNull();
+    });
+  });
+
   describe('tlsSkipVerify wiring', () => {
     it('returns false when env is unset even if config requests skip', () => {
       expect(shouldSkipTlsVerify(true, 'OAuth2 detection (test)')).toBe(false);

src/server/index.ts

@@ -211,6 +211,18 @@ class CapaServer {
 
 
   private async handleRequest(request: Request, server: any): Promise<Response> {
+    try {
+      return await this._handleRequest(request, server);
+    } catch (error: any) {
+      this.logger.failure(`Unhandled error in request handler: ${error?.message ?? error}`);
+      return new Response(
+        JSON.stringify({ error: 'Internal server error' }),
+        { status: 500, headers: { 'Content-Type': 'application/json' } }
+      );
+    }
+  }
+
+  private async _handleRequest(request: Request, server: any): Promise<Response> {
     const url = new URL(request.url);
     const path = url.pathname;
 
@@ -701,17 +713,29 @@ class CapaServer {
         );
       }
 
-      const tools = await mcpServer.listServerTools(serverId, capabilities);
+      // throwOnError surfaces connection/auth failures instead of silently
+      // returning an empty list, so the UI can distinguish "server has no tools"
+      // (200 + []) from "server unreachable" (502 + error).
+      const tools = await mcpServer.listServerTools(serverId, capabilities, { throwOnError: true });
       apiLogger.success(`Found ${tools.length} tools for server ${serverId}`);
       return new Response(
         JSON.stringify({ tools }),
         { headers: { 'Content-Type': 'application/json' } }
       );
     } catch (error: any) {
-      apiLogger.failure(`Error: ${error.message}`);
+      const detail = error?.message ?? String(error);
+      apiLogger.failure(`Error listing tools for ${serverId}: ${detail}`);
+      // Return a controlled, sanitized message — never echo raw exception text
+      // (which CodeQL flags as potential stack-trace exposure) back to the
+      // client. Full detail is logged above. An OAuth-disconnected server is
+      // not "unreachable", so distinguish that case to prompt re-auth.
+      const needsAuth = /authentication failed|reconnect oauth2/i.test(detail);
+      const message = needsAuth
+        ? `Authentication required for "${serverId}". Please reconnect this server's OAuth2 connection.`
+        : `Server unreachable: "${serverId}" could not be contacted.`;
       return new Response(
-        JSON.stringify({ error: error.message }),
-        { status: 500, headers: { 'Content-Type': 'application/json' } }
+        JSON.stringify({ error: message }),
+        { status: 502, headers: { 'Content-Type': 'application/json' } }
       );
     }
   }
@@ -1213,57 +1237,71 @@ class CapaServer {
   private async handleGetOAuth2Servers(projectId: string): Promise<Response> {
     const apiLogger = this.logger.child('API');
     apiLogger.info(`Get OAuth2 servers for project: ${projectId}`);
-    const capabilities = this.sessionManager.getProjectCapabilities(projectId);
-    if (!capabilities) {
-      return new Response(
-        JSON.stringify({ error: 'Project not configured' }),
-        { status: 404, headers: { 'Content-Type': 'application/json' } }
-      );
-    }
+    try {
+      const capabilities = this.sessionManager.getProjectCapabilities(projectId);
+      if (!capabilities) {
+        return new Response(
+          JSON.stringify({ error: 'Project not configured' }),
+          { status: 404, headers: { 'Content-Type': 'application/json' } }
+        );
+      }
 
-    // Ensure URL-based servers that require OAuth have def.oauth2 set (on-demand detection)
-    let capabilitiesUpdated = false;
-    for (const server of capabilities.servers) {
-      const hasExplicitAuthOnDemand = server.def.headers &&
-        Object.keys(server.def.headers).some(k => k.toLowerCase() === 'authorization');
-      if (server.def.url && !server.def.oauth2 && !hasExplicitAuthOnDemand) {
-        const oauth2Config = await this.oauth2Manager.detectOAuth2Requirement(server.def.url, { tlsSkipVerify: server.def.tlsSkipVerify });
-        if (oauth2Config) {
-          apiLogger.debug(`OAuth2 detected for ${server.id} (on-demand)`);
-          server.def.oauth2 = oauth2Config;
-          capabilitiesUpdated = true;
+      // Ensure URL-based servers that require OAuth have def.oauth2 set (on-demand detection)
+      let capabilitiesUpdated = false;
+      for (const server of capabilities.servers) {
+        const hasExplicitAuthOnDemand = server.def.headers &&
+          Object.keys(server.def.headers).some(k => k.toLowerCase() === 'authorization');
+        if (server.def.url && !server.def.oauth2 && !hasExplicitAuthOnDemand) {
+          try {
+            const oauth2Config = await this.oauth2Manager.detectOAuth2Requirement(server.def.url, { tlsSkipVerify: server.def.tlsSkipVerify });
+            if (oauth2Config) {
+              apiLogger.debug(`OAuth2 detected for ${server.id} (on-demand)`);
+              server.def.oauth2 = oauth2Config;
+              capabilitiesUpdated = true;
+            }
+          } catch (detectionError: any) {
+            apiLogger.warn(`OAuth2 detection failed for ${server.id}: ${detectionError?.message ?? detectionError}`);
+          }
         }
       }
-    }
-    if (capabilitiesUpdated) {
-      this.sessionManager.setProjectCapabilities(projectId, capabilities);
-    }
+      if (capabilitiesUpdated) {
+        this.sessionManager.setProjectCapabilities(projectId, capabilities);
+      }
 
-    const oauth2Servers = capabilities.servers
-      .filter((s: any) => s.def.oauth2)
-      .map((s: MCPServer) => {
-        const isConnected = this.oauth2Manager.isServerConnected(projectId, s.id);
-        let expiresAt: number | undefined;
-
-        if (isConnected) {
-          const tokenData = this.db.getOAuthToken(projectId, s.id);
-          expiresAt = tokenData?.expires_at ?? undefined;
-        }
-
-        return {
-          serverId: s.id,
-          serverUrl: s.def.url,
-          displayName: s.displayName ?? s.id,
-          isConnected: isConnected,
-          expiresAt: expiresAt,
-          oauth2Config: s.def.oauth2,
-        };
-      });
+      const oauth2Servers = capabilities.servers
+        .filter((s: any) => s.def.oauth2)
+        .map((s: MCPServer) => {
+          const isConnected = this.oauth2Manager.isServerConnected(projectId, s.id);
+          let expiresAt: number | undefined;
 
-    return new Response(
-      JSON.stringify({ servers: oauth2Servers }),
-      { headers: { 'Content-Type': 'application/json' } }
-    );
+          if (isConnected) {
+            const tokenData = this.db.getOAuthToken(projectId, s.id);
+            expiresAt = tokenData?.expires_at ?? undefined;
+          }
+
+          return {
+            serverId: s.id,
+            serverUrl: s.def.url,
+            displayName: s.displayName ?? s.id,
+            isConnected: isConnected,
+            expiresAt: expiresAt,
+            oauth2Config: s.def.oauth2,
+          };
+        });
+
+      return new Response(
+        JSON.stringify({ servers: oauth2Servers }),
+        { headers: { 'Content-Type': 'application/json' } }
+      );
+    } catch (error: any) {
+      // Log full detail server-side, but return a generic message so raw
+      // exception text (stack-trace exposure) never reaches the client.
+      apiLogger.failure(`Error getting OAuth2 servers: ${error?.message ?? error}`);
+      return new Response(
+        JSON.stringify({ error: 'Failed to load OAuth2 servers' }),
+        { status: 500, headers: { 'Content-Type': 'application/json' } }
+      );
+    }
   }
 
   private uiOrigin(): string {
@@ -2218,6 +2256,19 @@ class CapaServer {
 // Main
 const server = new CapaServer();
 
+// Safety net for stray async failures. The MCP SDK's HTTP/stdio transports keep
+// background sockets open; when a remote server becomes unreachable (e.g. VPN
+// dropped) those can reject after we've already returned a response, which Bun
+// would otherwise print as a raw "The socket connection was closed unexpectedly"
+// error. Log these instead of letting them crash the process or leak to stderr.
+process.on('unhandledRejection', (reason) => {
+  const message = reason instanceof Error ? reason.message : String(reason);
+  logger.warn(`Unhandled promise rejection (ignored): ${message}`);
+});
+process.on('uncaughtException', (error) => {
+  logger.error(`Uncaught exception (ignored): ${error?.message ?? error}`);
+});
+
 // Handle shutdown signals
 process.on('SIGTERM', () => server.stop());
 process.on('SIGINT', () => server.stop());

src/server/mcp-handler.ts

@@ -685,10 +685,14 @@ export class CapaMCPServer {
    * List tools available on a specific MCP server by ID.
    * Returns the raw MCP tool list (name, description, inputSchema).
    */
-  async listServerTools(serverId: string, capabilities: Capabilities): Promise<any[]> {
+  async listServerTools(
+    serverId: string,
+    capabilities: Capabilities,
+    options: { throwOnError?: boolean } = {},
+  ): Promise<any[]> {
     const serverDef = capabilities.servers.find((s) => s.id === serverId);
     if (!serverDef) return [];
-    return await this.mcpProxy.listTools(serverId, serverDef.def);
+    return await this.mcpProxy.listTools(serverId, serverDef.def, options);
   }
 
   /**