diff --git a/skills/capabilities-manager/references/capabilities-schema.md b/skills/capabilities-manager/references/capabilities-schema.md index 3507786..e79d4cd 100644 --- a/skills/capabilities-manager/references/capabilities-schema.md +++ b/skills/capabilities-manager/references/capabilities-schema.md @@ -90,7 +90,7 @@ Controls how capa exposes skill tools to the MCP client. Three modes: | `'none'` | Empty list | **No** — capa skips all project-local MCP config files (`.mcp.json`, `.cursor/mcp.json`, `.codex/config.toml` `mcp_servers.capa`, sub-agent `capa-` entries). Any previously-written entries are removed on install. | The agent must use `capa sh [--args]` (see [`commands.md`](./commands.md)). Sub-agent instruction files are still installed for documentation but their tools are not reachable over MCP. | Notes on `'none'`: -- The capa HTTP server still runs and the project endpoints stay live; `tools/list` just returns empty and `tools/call` rejects with a hint to use `capa sh`. +- The capa HTTP server still runs and the project endpoints stay live; `tools/list` returns empty so MCP-aware agents don't try to discover tools through capa's MCP endpoint. `tools/call` is **not** gated — that's the path `capa sh` uses to execute tools, and gating it would mean rejecting `capa sh` itself. - Useful when the user prefers to keep `.mcp.json` clean or has policy restrictions against writing per-project MCP configs. - Switching to `'none'` on a project that previously installed under another mode cleans up the old entries on the next `capa install`. diff --git a/src/cli/commands/install-tasks/configure-tools.ts b/src/cli/commands/install-tasks/configure-tools.ts index 8c6f8aa..d1da656 100644 --- a/src/cli/commands/install-tasks/configure-tools.ts +++ b/src/cli/commands/install-tasks/configure-tools.ts @@ -109,12 +109,20 @@ export function configureToolsTask(): Task { } } - const unexposed = getUnexposedToolIds(ctx.capabilitiesToUse); - if (unexposed.length > 0) { - ctx.warnings.push( - `${unexposed.length} tool(s) are not exposed to MCP clients (not required by any skill): ` + - `${unexposed.sort().join(', ')}. Add them to a skill's \`requires\` list to expose.`, - ); + // The "tool is not required by any skill" check is meaningless under + // `toolExposure: 'none'` — capa never exposes any tools to MCP clients + // in that mode by design (the agent invokes them via `capa sh`), so + // `requires` lists don't gate anything. Suppress the warning to avoid + // noise that would push users to "fix" a non-issue. + const toolExposure = ctx.capabilitiesToUse.options?.toolExposure; + if (toolExposure !== 'none') { + const unexposed = getUnexposedToolIds(ctx.capabilitiesToUse); + if (unexposed.length > 0) { + ctx.warnings.push( + `${unexposed.length} tool(s) are not exposed to MCP clients (not required by any skill): ` + + `${unexposed.sort().join(', ')}. Add them to a skill's \`requires\` list to expose.`, + ); + } } }, }; diff --git a/src/server/__tests__/mcp-handler.integration.test.ts b/src/server/__tests__/mcp-handler.integration.test.ts index e9c1b5e..f395d68 100644 --- a/src/server/__tests__/mcp-handler.integration.test.ts +++ b/src/server/__tests__/mcp-handler.integration.test.ts @@ -38,6 +38,7 @@ function makeHarness(initial: Capabilities): Harness { } function destroyHarness(h: Harness): void { + h.sessionManager.dispose(); h.db.close(); try { rmSync(h.tempDir, { recursive: true, force: true }); @@ -413,20 +414,32 @@ describe('handleMessage > toolExposure: none', () => { expect(resp.result?.tools).toEqual([]); }); - it('rejects every tools/call (including setup_tools / call_tool) with a capa-sh hint', async () => { - for (const name of ['setup_tools', 'call_tool', 't', 'whatever']) { - const resp = await h.mcp.handleMessage({ - jsonrpc: '2.0', - id: 1, - method: 'tools/call', - params: { name, arguments: {} }, - }); - expect(resp.error).toBeUndefined(); - expect(resp.result?.isError).toBe(true); - const payload = parseToolText(resp.result); - expect(payload.error).toMatch(/toolExposure: none/); - expect(payload.error).toMatch(/capa sh/); - } + // `tools/list` hides tools from MCP-aware agents (so they don't try to + // discover them through capa's MCP endpoint), but `tools/call` is *not* + // gated on `toolExposure`. The `capa sh` CLI is the documented escape + // hatch for this mode and uses this exact endpoint as its execution + // channel — gating it here would mean rejecting `capa sh` itself. + it('executes a configured tool by qualified name (capa sh fallback path)', async () => { + const resp = await h.mcp.handleMessage({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 't', arguments: {} }, + }); + expect(resp.error).toBeUndefined(); + expect(resp.result?.isError).toBeFalsy(); + expect(resp.result?.content).toBeDefined(); + }); + + it('returns the standard "Tool not found" error for unknown names', async () => { + const resp = await h.mcp.handleMessage({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'totally-made-up', arguments: {} }, + }); + expect(resp.error).toBeDefined(); + expect(resp.error?.message).toMatch(/Tool not found/); }); }); diff --git a/src/server/__tests__/session-manager.test.ts b/src/server/__tests__/session-manager.test.ts index 7857e05..d174104 100644 --- a/src/server/__tests__/session-manager.test.ts +++ b/src/server/__tests__/session-manager.test.ts @@ -27,6 +27,7 @@ describe('SessionManager', () => { }); afterEach(() => { + sessionManager.dispose(); db.close(); try { rmSync(tempDir, { recursive: true, force: true }); diff --git a/src/server/index.ts b/src/server/index.ts index 328137f..a6fda7d 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -1982,6 +1982,7 @@ class CapaServer { this.oauthCallbackServers.clear(); // Close database + this.sessionManager.dispose(); this.db.close(); this.logger.success('CAPA server stopped'); diff --git a/src/server/mcp-handler.ts b/src/server/mcp-handler.ts index f4568a3..6d97f62 100644 --- a/src/server/mcp-handler.ts +++ b/src/server/mcp-handler.ts @@ -328,26 +328,6 @@ export class CapaMCPServer { const capabilities = this.sessionManager.getProjectCapabilities(this.projectId); const toolExposureMode = capabilities?.options?.toolExposure || 'expose-all'; - // Tool exposure disabled — instruct the agent to use the CLI fallback - // instead of silently failing. This branch should rarely fire because - // capa skips writing MCP entries in `'none'` mode, but a user who - // hand-wires the endpoint shouldn't hit a confusing generic error. - if (toolExposureMode === 'none') { - return { - content: [ - { - type: 'text', - text: JSON.stringify({ - error: - 'Tool exposure is disabled for this project (toolExposure: none). ' + - 'Use the `capa sh` CLI to discover and invoke tools instead.', - }), - }, - ], - isError: true, - }; - } - // Handle setup_tools if (name === 'setup_tools' && toolExposureMode === 'on-demand') { return await this.handleSetupTools(args as { skills: string[] }); @@ -1095,33 +1075,6 @@ export class CapaMCPServer { this.logger.info(`Call tool: ${name}`); this.logger.debug(`Arguments: ${JSON.stringify(args)}`); - // Tool exposure disabled — reject before doing any per-tool lookup. - // Capa skips MCP file writes in `'none'` mode, but a hand-wired endpoint - // should still get a helpful nudge toward the CLI fallback. - { - const caps = this.sessionManager.getProjectCapabilities(this.projectId); - if (caps?.options?.toolExposure === 'none') { - this.logger.warn(`tools/call hit on a project with toolExposure: none (tool=${name})`); - return { - jsonrpc: '2.0', - id: message.id, - result: { - content: [ - { - type: 'text', - text: JSON.stringify({ - error: - 'Tool exposure is disabled for this project (toolExposure: none). ' + - 'Use the `capa sh` CLI to discover and invoke tools instead.', - }), - }, - ], - isError: true, - }, - }; - } - } - // Handle setup_tools if (name === 'setup_tools') { try { diff --git a/src/server/session-manager.ts b/src/server/session-manager.ts index 05ea86e..16a228a 100644 --- a/src/server/session-manager.ts +++ b/src/server/session-manager.ts @@ -18,6 +18,8 @@ export class SessionManager { private sessions = new Map(); private projectCapabilities = new Map(); private capabilitiesLoadInflight = new Map>(); + private cleanupTimer: ReturnType | null = null; + private disposed = false; private logger = logger.child('SessionManager'); constructor(db: CapaDatabase) { @@ -25,6 +27,23 @@ export class SessionManager { this.startCleanupTimer(); } + /** + * Stop background work (cleanup timer) and release references. + * + * Idempotent. Safe to call multiple times. Must be called by tests and by + * the production graceful-shutdown path before closing the underlying + * database; otherwise the cleanup interval will keep a strong reference to + * `this` (and the closed DB) and fire after teardown. + */ + dispose(): void { + if (this.disposed) return; + this.disposed = true; + if (this.cleanupTimer) { + clearInterval(this.cleanupTimer); + this.cleanupTimer = null; + } + } + /** * Create a new session */ @@ -288,14 +307,27 @@ export class SessionManager { * Clean up expired sessions */ private startCleanupTimer(): void { - setInterval(() => { + const timer = setInterval(() => { this.cleanupExpiredSessions(); }, 60000); // Run every minute + // Don't keep the event loop alive solely for this cleanup task. This is a + // no-op on platforms/runtimes that don't support `unref` on timers. + (timer as { unref?: () => void }).unref?.(); + this.cleanupTimer = timer; } private cleanupExpiredSessions(): void { + if (this.disposed) return; const timeout = 60; // 60 minutes - this.db.deleteExpiredSessions(timeout); + try { + this.db.deleteExpiredSessions(timeout); + } catch (error) { + // The database may have been closed between scheduling and the timer + // firing (e.g. during shutdown or in tests). Swallow the error rather + // than crash the process with an unhandled rejection. + this.logger.debug(`Skipping session cleanup: ${(error as Error).message}`); + return; + } // Clean up in-memory sessions const cutoff = Date.now() - timeout * 60 * 1000;