From 7b1083a98fcb5c5de4ae4770dd83c4824c415751 Mon Sep 17 00:00:00 2001 From: execsumo <168054536+execsumo@users.noreply.github.com> Date: Wed, 25 Feb 2026 20:12:08 -0800 Subject: [PATCH 1/4] fix: bypass CORS for MCP HTTP/SSE server testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace browser-based SDK transports (SSEClientTransport, StreamableHTTPClientTransport) with a custom NodeHttpTransport that uses Node.js native http/https modules. This bypasses CORS restrictions in Obsidian's Electron renderer, which sends Origin: app://obsidian.md with every request — an origin that MCP servers never whitelist. This affects all HTTP and SSE servers (local and remote), not just local ones. Stdio servers are unaffected. The 10-second timeout, custom header passthrough, and error handling behavior are all preserved. --- src/core/mcp/McpTester.ts | 114 +++++++++++++++++++++++-- tests/integration/core/mcp/mcp.test.ts | 53 +++++++++--- tests/unit/core/mcp/McpTester.test.ts | 70 +++++++-------- 3 files changed, 178 insertions(+), 59 deletions(-) diff --git a/src/core/mcp/McpTester.ts b/src/core/mcp/McpTester.ts index a1ddec07..d401fe64 100644 --- a/src/core/mcp/McpTester.ts +++ b/src/core/mcp/McpTester.ts @@ -1,7 +1,8 @@ +import * as http from 'http'; +import * as https from 'https'; + import { Client } from '@modelcontextprotocol/sdk/client'; -import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio'; -import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp'; import { getEnhancedPath } from '../../utils/env'; import { parseCommand } from '../../utils/mcp'; @@ -27,6 +28,110 @@ interface UrlServerConfig { headers?: Record; } +/** + * Custom MCP transport using Node.js native http/https modules. + * Bypasses browser CORS restrictions that block Obsidian's Electron renderer + * (Origin: app://obsidian.md) from connecting to MCP servers. + */ +class NodeHttpTransport { + private _url: URL; + private _headers: Record; + private _sessionId?: string; + + // Transport interface callbacks + onmessage?: (message: unknown) => void; + onerror?: (error: Error) => void; + onclose?: () => void; + + constructor(url: URL, headers?: Record) { + this._url = url; + this._headers = headers ?? {}; + } + + async start(): Promise { + // Nothing to do on start — the Client will send initialize via send() + } + + async send(message: unknown): Promise { + const body = JSON.stringify(message); + const mod = this._url.protocol === 'https:' ? https : http; + + const headers: Record = { + ...this._headers, + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', + }; + if (this._sessionId) { + headers['mcp-session-id'] = this._sessionId; + } + + return new Promise((resolve, reject) => { + const req = mod.request( + this._url, + { method: 'POST', headers }, + (res: http.IncomingMessage) => { + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => { + const sessionHeader = res.headers['mcp-session-id']; + if (sessionHeader) { + this._sessionId = Array.isArray(sessionHeader) ? sessionHeader[0] : sessionHeader; + } + + if (res.statusCode && (res.statusCode < 200 || res.statusCode >= 300)) { + reject(new Error(`HTTP ${res.statusCode}`)); + return; + } + + const text = Buffer.concat(chunks).toString('utf-8').trim(); + if (!text) { + resolve(); + return; + } + + // Handle SSE-formatted responses (content-type: text/event-stream) + const contentType = res.headers['content-type'] ?? ''; + if (contentType.includes('text/event-stream')) { + const lines = text.split('\n'); + for (const line of lines) { + if (line.startsWith('data: ')) { + const data = line.slice(6).trim(); + if (data) { + try { + this.onmessage?.(JSON.parse(data)); + } catch { + // Skip unparseable SSE data lines + } + } + } + } + resolve(); + return; + } + + // Handle JSON response + try { + this.onmessage?.(JSON.parse(text)); + resolve(); + } catch { + reject(new Error('Invalid JSON response')); + } + }); + res.on('error', (err: Error) => reject(err)); + }, + ); + + req.on('error', (err: Error) => reject(err)); + req.write(body); + req.end(); + }); + } + + async close(): Promise { + this.onclose?.(); + } +} + export async function testMcpServer(server: ClaudianMcpServer): Promise { const type = getMcpServerType(server.config); @@ -47,10 +152,7 @@ export async function testMcpServer(server: ClaudianMcpServer): Promise ({ StdioClientTransport: jest.fn(), })); -jest.mock('@modelcontextprotocol/sdk/client/sse', () => ({ - SSEClientTransport: jest.fn(), +jest.mock('http', () => ({ + request: jest.fn(), })); -jest.mock('@modelcontextprotocol/sdk/client/streamableHttp', () => ({ - StreamableHTTPClientTransport: jest.fn(), +jest.mock('https', () => ({ + request: jest.fn(), })); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const mockHttp = require('http') as { request: jest.Mock }; +// eslint-disable-next-line @typescript-eslint/no-require-imports +const mockHttps = require('https') as { request: jest.Mock }; + +function createMockResponse(statusCode: number, body: string, headers: Record = {}): EventEmitter { + const res = new EventEmitter() as EventEmitter & { statusCode: number; headers: Record }; + res.statusCode = statusCode; + res.headers = { 'content-type': 'application/json', ...headers }; + process.nextTick(() => { + res.emit('data', Buffer.from(body)); + res.emit('end'); + }); + return res; +} + +function createMockRequest(): EventEmitter & { write: jest.Mock; end: jest.Mock } { + const req = new EventEmitter() as EventEmitter & { write: jest.Mock; end: jest.Mock }; + req.write = jest.fn(); + req.end = jest.fn(); + return req; +} + +function setupHttpMock(mod: { request: jest.Mock }, statusCode = 200, body = '', headers: Record = {}) { + const req = createMockRequest(); + mod.request.mockImplementation((_url: unknown, _opts: unknown, cb: (res: unknown) => void) => { + const res = createMockResponse(statusCode, body, headers); + cb(res); + return req; + }); + return req; +} + function createMemoryStorage(initialFile?: Record): { storage: McpStorage; files: Map; @@ -660,10 +693,6 @@ describe('McpTester', () => { expect(result.serverName).toBe('test-srv'); expect(result.serverVersion).toBe('1.0.0'); expect(result.tools).toEqual([{ name: 'tool-a', description: 'Tool A', inputSchema: { type: 'object' } }]); - expect(StreamableHTTPClientTransport).toHaveBeenCalledWith( - expect.any(URL), - expect.objectContaining({ requestInit: { headers: { Authorization: 'token' } } }), - ); }); it('should test sse server and return tools', async () => { @@ -680,10 +709,6 @@ describe('McpTester', () => { expect(result.serverName).toBe('test-srv'); expect(result.serverVersion).toBe('1.0.0'); expect(result.tools).toEqual([{ name: 'tool-a', description: 'Tool A', inputSchema: { type: 'object' } }]); - expect(SSEClientTransport).toHaveBeenCalledWith( - expect.any(URL), - expect.objectContaining({ requestInit: { headers: { Authorization: 'token' } } }), - ); }); it('should return failure when connect fails', async () => { diff --git a/tests/unit/core/mcp/McpTester.test.ts b/tests/unit/core/mcp/McpTester.test.ts index 28543c0f..29c8aa71 100644 --- a/tests/unit/core/mcp/McpTester.test.ts +++ b/tests/unit/core/mcp/McpTester.test.ts @@ -1,7 +1,7 @@ import { testMcpServer } from '@/core/mcp/McpTester'; import type { ClaudianMcpServer } from '@/core/types'; -// Mock the MCP SDK transports and client +// Mock the MCP SDK client jest.mock('@modelcontextprotocol/sdk/client', () => ({ Client: jest.fn().mockImplementation(() => ({ connect: jest.fn(), @@ -16,18 +16,10 @@ jest.mock('@modelcontextprotocol/sdk/client', () => ({ })), })); -jest.mock('@modelcontextprotocol/sdk/client/sse', () => ({ - SSEClientTransport: jest.fn(), -})); - jest.mock('@modelcontextprotocol/sdk/client/stdio', () => ({ StdioClientTransport: jest.fn(), })); -jest.mock('@modelcontextprotocol/sdk/client/streamableHttp', () => ({ - StreamableHTTPClientTransport: jest.fn(), -})); - jest.mock('@/utils/env', () => ({ getEnhancedPath: jest.fn((p?: string) => p || '/usr/bin'), })); @@ -40,6 +32,15 @@ jest.mock('@/utils/mcp', () => ({ }), })); +// Mock http/https to prevent real network requests from the NodeHttpTransport +jest.mock('http', () => ({ + request: jest.fn(), +})); + +jest.mock('https', () => ({ + request: jest.fn(), +})); + describe('testMcpServer', () => { beforeEach(() => { jest.clearAllMocks(); @@ -88,13 +89,14 @@ describe('testMcpServer', () => { it('should connect to an SSE server', async () => { const server: ClaudianMcpServer = { name: 'sse-test', - config: { type: 'sse' as const, url: 'https://example.com/sse' }, + config: { type: 'sse' as const, url: 'http://example.com/sse' }, enabled: true, contextSaving: false, }; const result = await testMcpServer(server); + // Client is mocked, so connect succeeds without making real HTTP requests expect(result.success).toBe(true); expect(result.tools).toHaveLength(2); }); @@ -104,6 +106,19 @@ describe('testMcpServer', () => { it('should connect to an HTTP server', async () => { const server: ClaudianMcpServer = { name: 'http-test', + config: { type: 'http' as const, url: 'http://example.com/api' }, + enabled: true, + contextSaving: false, + }; + + const result = await testMcpServer(server); + + expect(result.success).toBe(true); + }); + + it('should connect to an HTTPS server', async () => { + const server: ClaudianMcpServer = { + name: 'https-test', config: { type: 'http' as const, url: 'https://example.com/api' }, enabled: true, contextSaving: false, @@ -119,7 +134,7 @@ describe('testMcpServer', () => { name: 'http-auth', config: { type: 'http' as const, - url: 'https://example.com/api', + url: 'http://example.com/api', headers: { Authorization: 'Bearer token' }, }, enabled: true, @@ -128,20 +143,16 @@ describe('testMcpServer', () => { const result = await testMcpServer(server); + // Transport is created with headers; Client is mocked so connect succeeds expect(result.success).toBe(true); }); }); describe('error handling', () => { - it('should return error when transport creation fails', async () => { - const { SSEClientTransport } = jest.requireMock('@modelcontextprotocol/sdk/client/sse'); - SSEClientTransport.mockImplementationOnce(() => { - throw new Error('Transport init failed'); - }); - + it('should return error for invalid URL', async () => { const server: ClaudianMcpServer = { - name: 'bad-sse', - config: { type: 'sse' as const, url: 'https://example.com/sse' }, + name: 'bad-url', + config: { type: 'http' as const, url: 'not-a-valid-url' }, enabled: true, contextSaving: false, }; @@ -149,29 +160,10 @@ describe('testMcpServer', () => { const result = await testMcpServer(server); expect(result.success).toBe(false); - expect(result.error).toBe('Transport init failed'); + expect(result.error).toBeDefined(); expect(result.tools).toEqual([]); }); - it('should return generic error for non-Error transport failures', async () => { - const { StreamableHTTPClientTransport } = jest.requireMock('@modelcontextprotocol/sdk/client/streamableHttp'); - StreamableHTTPClientTransport.mockImplementationOnce(() => { - throw 'string error'; // eslint-disable-line no-throw-literal - }); - - const server: ClaudianMcpServer = { - name: 'bad-http', - config: { type: 'http' as const, url: 'https://example.com' }, - enabled: true, - contextSaving: false, - }; - - const result = await testMcpServer(server); - - expect(result.success).toBe(false); - expect(result.error).toBe('Invalid server configuration'); - }); - it('should return error when connection fails', async () => { const { Client } = jest.requireMock('@modelcontextprotocol/sdk/client'); Client.mockImplementationOnce(() => ({ From 51bed4b02b7d016db7c3ae8956b47ef9c7ad5a24 Mon Sep 17 00:00:00 2001 From: execsumo <168054536+execsumo@users.noreply.github.com> Date: Fri, 27 Feb 2026 19:07:09 -0800 Subject: [PATCH 2/4] fix: resolve lint errors in mcp tester and tests --- src/core/mcp/McpTester.ts | 5 ++-- tests/integration/core/mcp/mcp.test.ts | 33 +------------------------- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/src/core/mcp/McpTester.ts b/src/core/mcp/McpTester.ts index d401fe64..1cbffd87 100644 --- a/src/core/mcp/McpTester.ts +++ b/src/core/mcp/McpTester.ts @@ -1,8 +1,7 @@ -import * as http from 'http'; -import * as https from 'https'; - import { Client } from '@modelcontextprotocol/sdk/client'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio'; +import * as http from 'http'; +import * as https from 'https'; import { getEnhancedPath } from '../../utils/env'; import { parseCommand } from '../../utils/mcp'; diff --git a/tests/integration/core/mcp/mcp.test.ts b/tests/integration/core/mcp/mcp.test.ts index b765c3d6..8829d275 100644 --- a/tests/integration/core/mcp/mcp.test.ts +++ b/tests/integration/core/mcp/mcp.test.ts @@ -1,5 +1,3 @@ -import { EventEmitter } from 'events'; - import { Client } from '@modelcontextprotocol/sdk/client'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio'; @@ -40,38 +38,9 @@ jest.mock('https', () => ({ request: jest.fn(), })); -// eslint-disable-next-line @typescript-eslint/no-require-imports -const mockHttp = require('http') as { request: jest.Mock }; -// eslint-disable-next-line @typescript-eslint/no-require-imports -const mockHttps = require('https') as { request: jest.Mock }; - -function createMockResponse(statusCode: number, body: string, headers: Record = {}): EventEmitter { - const res = new EventEmitter() as EventEmitter & { statusCode: number; headers: Record }; - res.statusCode = statusCode; - res.headers = { 'content-type': 'application/json', ...headers }; - process.nextTick(() => { - res.emit('data', Buffer.from(body)); - res.emit('end'); - }); - return res; -} -function createMockRequest(): EventEmitter & { write: jest.Mock; end: jest.Mock } { - const req = new EventEmitter() as EventEmitter & { write: jest.Mock; end: jest.Mock }; - req.write = jest.fn(); - req.end = jest.fn(); - return req; -} -function setupHttpMock(mod: { request: jest.Mock }, statusCode = 200, body = '', headers: Record = {}) { - const req = createMockRequest(); - mod.request.mockImplementation((_url: unknown, _opts: unknown, cb: (res: unknown) => void) => { - const res = createMockResponse(statusCode, body, headers); - cb(res); - return req; - }); - return req; -} + function createMemoryStorage(initialFile?: Record): { storage: McpStorage; From 1f5b251ba149bfd7a11d2a26c3483c7ee94855e2 Mon Sep 17 00:00:00 2001 From: execsumo <168054536+execsumo@users.noreply.github.com> Date: Fri, 27 Feb 2026 19:40:56 -0800 Subject: [PATCH 3/4] feat: load global user-level skills and MCP servers - SkillStorage: load skills from ~/.claude/skills/ alongside vault skills (.claude/skills/). Vault skills take precedence on name collision. Global skills get id prefix 'skill-global-'. - McpStorage: load MCP servers from ~/.claude/settings.json alongside vault .claude/mcp.json. Vault servers take precedence on name collision. Restructured load() to isolate vault try/catch from global merge. - AgentManager already loads global agents from ~/.claude/agents - no changes needed. - Added unit tests for both global loading paths (5 new SkillStorage tests, 6 new McpStorage tests) with fs mocks. --- src/core/storage/McpStorage.ts | 104 +++++++++++---- src/core/storage/SkillStorage.ts | 49 ++++++- src/core/storage/index.ts | 4 +- tests/unit/core/storage/McpStorage.test.ts | 101 +++++++++++++- tests/unit/core/storage/SkillStorage.test.ts | 133 ++++++++++++++++++- 5 files changed, 358 insertions(+), 33 deletions(-) diff --git a/src/core/storage/McpStorage.ts b/src/core/storage/McpStorage.ts index 053bf501..5e4c1a29 100644 --- a/src/core/storage/McpStorage.ts +++ b/src/core/storage/McpStorage.ts @@ -3,6 +3,8 @@ * * MCP server configurations are stored in Claude Code-compatible format * with optional Claudian-specific metadata in _claudian field. + * Also loads read-only servers from the user-level ~/.claude/settings.json + * (vault servers take precedence on name collision). * * File format: * { @@ -17,6 +19,10 @@ * } */ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + import type { ClaudianMcpConfigFile, ClaudianMcpServer, @@ -29,49 +35,93 @@ import type { VaultFileAdapter } from './VaultFileAdapter'; /** Path to MCP config file relative to vault root. */ export const MCP_CONFIG_PATH = '.claude/mcp.json'; +/** Absolute path to the user-level Claude Code settings file containing global MCP servers. */ +export const GLOBAL_MCP_SETTINGS_PATH = path.join(os.homedir(), '.claude', 'settings.json'); + export class McpStorage { - constructor(private adapter: VaultFileAdapter) {} + constructor(private adapter: VaultFileAdapter) { } async load(): Promise { + let vaultServers: ClaudianMcpServer[] = []; + try { if (!(await this.adapter.exists(MCP_CONFIG_PATH))) { - return []; + // Fall through to global loading below + } else { + const content = await this.adapter.read(MCP_CONFIG_PATH); + const file = JSON.parse(content) as ClaudianMcpConfigFile; + + if (file.mcpServers && typeof file.mcpServers === 'object') { + const claudianMeta = file._claudian?.servers ?? {}; + + for (const [name, config] of Object.entries(file.mcpServers)) { + if (!isValidMcpServerConfig(config)) { + continue; + } + + const meta = claudianMeta[name] ?? {}; + const disabledTools = Array.isArray(meta.disabledTools) + ? meta.disabledTools.filter((tool) => typeof tool === 'string') + : undefined; + const normalizedDisabledTools = + disabledTools && disabledTools.length > 0 ? disabledTools : undefined; + + vaultServers.push({ + name, + config, + enabled: meta.enabled ?? DEFAULT_MCP_SERVER.enabled, + contextSaving: meta.contextSaving ?? DEFAULT_MCP_SERVER.contextSaving, + disabledTools: normalizedDisabledTools, + description: meta.description, + }); + } + } } + } catch { + // Non-critical: return whatever vault servers could be loaded + } - const content = await this.adapter.read(MCP_CONFIG_PATH); - const file = JSON.parse(content) as ClaudianMcpConfigFile; - - if (!file.mcpServers || typeof file.mcpServers !== 'object') { - return []; + // Append global servers outside the vault try/catch (vault wins on name collision) + const vaultNames = new Set(vaultServers.map(s => s.name)); + for (const globalServer of this.loadGlobal()) { + if (!vaultNames.has(globalServer.name)) { + vaultServers.push(globalServer); } + } - const claudianMeta = file._claudian?.servers ?? {}; - const servers: ClaudianMcpServer[] = []; + return vaultServers; + } - for (const [name, config] of Object.entries(file.mcpServers)) { - if (!isValidMcpServerConfig(config)) { - continue; - } - const meta = claudianMeta[name] ?? {}; - const disabledTools = Array.isArray(meta.disabledTools) - ? meta.disabledTools.filter((tool) => typeof tool === 'string') - : undefined; - const normalizedDisabledTools = - disabledTools && disabledTools.length > 0 ? disabledTools : undefined; + /** + * Load MCP servers from the user-level ~/.claude/settings.json. + * These are read-only from Claudian's perspective (CC owns the file). + */ + private loadGlobal(): ClaudianMcpServer[] { + try { + if (!fs.existsSync(GLOBAL_MCP_SETTINGS_PATH)) return []; - servers.push({ + const content = fs.readFileSync(GLOBAL_MCP_SETTINGS_PATH, 'utf-8'); + const settings = JSON.parse(content) as Record; + + const mcpServers = settings.mcpServers; + if (!mcpServers || typeof mcpServers !== 'object' || Array.isArray(mcpServers)) { + return []; + } + + const result: ClaudianMcpServer[] = []; + for (const [name, config] of Object.entries(mcpServers as Record)) { + if (!isValidMcpServerConfig(config)) continue; + result.push({ name, - config, - enabled: meta.enabled ?? DEFAULT_MCP_SERVER.enabled, - contextSaving: meta.contextSaving ?? DEFAULT_MCP_SERVER.contextSaving, - disabledTools: normalizedDisabledTools, - description: meta.description, + config: config as McpServerConfig, + enabled: DEFAULT_MCP_SERVER.enabled, + contextSaving: DEFAULT_MCP_SERVER.contextSaving, }); } - - return servers; + return result; } catch { + // Non-critical: global settings file may be absent or malformed return []; } } diff --git a/src/core/storage/SkillStorage.ts b/src/core/storage/SkillStorage.ts index 689a783e..14a0e3ae 100644 --- a/src/core/storage/SkillStorage.ts +++ b/src/core/storage/SkillStorage.ts @@ -1,14 +1,20 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + import { parsedToSlashCommand, parseSlashCommandContent, serializeCommand } from '../../utils/slashCommand'; import type { SlashCommand } from '../types'; import type { VaultFileAdapter } from './VaultFileAdapter'; export const SKILLS_PATH = '.claude/skills'; +export const GLOBAL_SKILLS_DIR = path.join(os.homedir(), '.claude', 'skills'); export class SkillStorage { - constructor(private adapter: VaultFileAdapter) {} + constructor(private adapter: VaultFileAdapter) { } async loadAll(): Promise { const skills: SlashCommand[] = []; + const loadedNames = new Set(); try { const folders = await this.adapter.listFolders(SKILLS_PATH); @@ -28,17 +34,56 @@ export class SkillStorage { name: skillName, source: 'user', })); + loadedNames.add(skillName); } catch { // Non-critical: skip malformed skill files } } } catch { - return []; + return skills; } + // Also load user-level skills from ~/.claude/skills (global Claude Code skills). + // Vault skills take precedence: if a skill with the same name already loaded, skip. + this.loadGlobalSkills(skills, loadedNames); + return skills; } + private loadGlobalSkills(skills: SlashCommand[], loadedNames: Set): void { + try { + if (!fs.existsSync(GLOBAL_SKILLS_DIR)) return; + + const entries = fs.readdirSync(GLOBAL_SKILLS_DIR, { withFileTypes: true }); + + for (const entry of entries) { + if (!entry.isDirectory()) continue; + + const skillName = entry.name; + if (loadedNames.has(skillName)) continue; // Vault skill wins + + const skillPath = path.join(GLOBAL_SKILLS_DIR, skillName, 'SKILL.md'); + + try { + if (!fs.existsSync(skillPath)) continue; + + const content = fs.readFileSync(skillPath, 'utf-8'); + const parsed = parseSlashCommandContent(content); + + skills.push(parsedToSlashCommand(parsed, { + id: `skill-global-${skillName}`, + name: skillName, + source: 'user', + })); + } catch { + // Non-critical: skip malformed global skill files + } + } + } catch { + // Non-critical: global skills directory may be inaccessible + } + } + async save(skill: SlashCommand): Promise { const name = skill.name; const dirPath = `${SKILLS_PATH}/${name}`; diff --git a/src/core/storage/index.ts b/src/core/storage/index.ts index 4d23e17f..11c5e7b6 100644 --- a/src/core/storage/index.ts +++ b/src/core/storage/index.ts @@ -5,9 +5,9 @@ export { ClaudianSettingsStorage, type StoredClaudianSettings, } from './ClaudianSettingsStorage'; -export { MCP_CONFIG_PATH, McpStorage } from './McpStorage'; +export { GLOBAL_MCP_SETTINGS_PATH, MCP_CONFIG_PATH, McpStorage } from './McpStorage'; export { SESSIONS_PATH, SessionStorage } from './SessionStorage'; -export { SKILLS_PATH, SkillStorage } from './SkillStorage'; +export { GLOBAL_SKILLS_DIR, SKILLS_PATH, SkillStorage } from './SkillStorage'; export { COMMANDS_PATH, SlashCommandStorage } from './SlashCommandStorage'; export { CLAUDE_PATH, diff --git a/tests/unit/core/storage/McpStorage.test.ts b/tests/unit/core/storage/McpStorage.test.ts index 5aab6182..87dc776d 100644 --- a/tests/unit/core/storage/McpStorage.test.ts +++ b/tests/unit/core/storage/McpStorage.test.ts @@ -1,6 +1,11 @@ -import { McpStorage } from '@/core/storage'; +import * as fs from 'fs'; + +import { GLOBAL_MCP_SETTINGS_PATH, McpStorage } from '@/core/storage'; import type { VaultFileAdapter } from '@/core/storage/VaultFileAdapter'; +jest.mock('fs'); +const fsMock = fs as jest.Mocked; + /** Mock adapter with exposed store for test assertions. */ type MockAdapter = VaultFileAdapter & { _store: Record }; @@ -25,6 +30,12 @@ function createMockAdapter(files: Record = {}): MockAdapter { } describe('McpStorage', () => { + beforeEach(() => { + jest.clearAllMocks(); + // Default: ~/.claude/settings.json does not exist + fsMock.existsSync.mockReturnValue(false); + }); + describe('load', () => { it('returns empty array when file does not exist', async () => { const adapter = createMockAdapter(); @@ -615,4 +626,92 @@ describe('McpStorage', () => { expect(result).not.toBeNull(); }); }); + describe('global MCP servers', () => { + it('loads global servers from ~/.claude/settings.json', async () => { + const settings = JSON.stringify({ + mcpServers: { + 'global-srv': { command: 'global-cmd' }, + }, + }); + fsMock.existsSync.mockReturnValue(true); + fsMock.readFileSync.mockReturnValue(settings as any); + + const adapter = createMockAdapter({}); + const storage = new McpStorage(adapter); + const servers = await storage.load(); + + expect(servers).toHaveLength(1); + expect(servers[0].name).toBe('global-srv'); + expect(servers[0].enabled).toBe(true); + expect(servers[0].contextSaving).toBe(true); // DEFAULT_MCP_SERVER default + }); + + it('vault server wins over global server with the same name', async () => { + const vaultConfig = { + mcpServers: { shared: { command: 'vault-cmd' } }, + }; + const globalSettings = JSON.stringify({ + mcpServers: { shared: { command: 'global-cmd' } }, + }); + fsMock.existsSync.mockReturnValue(true); + fsMock.readFileSync.mockReturnValue(globalSettings as any); + + const adapter = createMockAdapter({ + '.claude/mcp.json': JSON.stringify(vaultConfig), + }); + const storage = new McpStorage(adapter); + const servers = await storage.load(); + + expect(servers).toHaveLength(1); + expect((servers[0].config as { command: string }).command).toBe('vault-cmd'); + }); + + it('does not fail when ~/.claude/settings.json does not exist', async () => { + fsMock.existsSync.mockReturnValue(false); + + const adapter = createMockAdapter({}); + const storage = new McpStorage(adapter); + const servers = await storage.load(); + + expect(servers).toEqual([]); + expect(fsMock.readFileSync).not.toHaveBeenCalled(); + }); + + it('handles missing mcpServers field in settings.json gracefully', async () => { + fsMock.existsSync.mockReturnValue(true); + fsMock.readFileSync.mockReturnValue(JSON.stringify({ permissions: {} }) as any); + + const adapter = createMockAdapter({}); + const storage = new McpStorage(adapter); + const servers = await storage.load(); + + expect(servers).toEqual([]); + }); + + it('merges global and vault servers when names differ', async () => { + const vaultConfig = { + mcpServers: { 'vault-only': { command: 'vault-cmd' } }, + }; + const globalSettings = JSON.stringify({ + mcpServers: { 'global-only': { command: 'global-cmd' } }, + }); + fsMock.existsSync.mockReturnValue(true); + fsMock.readFileSync.mockReturnValue(globalSettings as any); + + const adapter = createMockAdapter({ + '.claude/mcp.json': JSON.stringify(vaultConfig), + }); + const storage = new McpStorage(adapter); + const servers = await storage.load(); + + expect(servers).toHaveLength(2); + const names = servers.map(s => s.name).sort(); + expect(names).toEqual(['global-only', 'vault-only']); + }); + + it('exports GLOBAL_MCP_SETTINGS_PATH pointing to ~/.claude/settings.json', () => { + expect(GLOBAL_MCP_SETTINGS_PATH).toContain('.claude'); + expect(GLOBAL_MCP_SETTINGS_PATH).toContain('settings.json'); + }); + }); }); diff --git a/tests/unit/core/storage/SkillStorage.test.ts b/tests/unit/core/storage/SkillStorage.test.ts index 837ab9cb..f0d90023 100644 --- a/tests/unit/core/storage/SkillStorage.test.ts +++ b/tests/unit/core/storage/SkillStorage.test.ts @@ -1,6 +1,25 @@ -import { SKILLS_PATH,SkillStorage } from '@/core/storage/SkillStorage'; +import * as fs from 'fs'; + +import { GLOBAL_SKILLS_DIR, SKILLS_PATH, SkillStorage } from '@/core/storage/SkillStorage'; import type { VaultFileAdapter } from '@/core/storage/VaultFileAdapter'; +jest.mock('fs'); +const fsMock = fs as jest.Mocked; + +// Helper to build a dirent-like object +function makeDirent(name: string, isDir: boolean): fs.Dirent { + return { + name, + isDirectory: () => isDir, + isFile: () => !isDir, + isBlockDevice: () => false, + isCharacterDevice: () => false, + isFIFO: () => false, + isSocket: () => false, + isSymbolicLink: () => false, + } as unknown as fs.Dirent; +} + function createMockAdapter(files: Record = {}): VaultFileAdapter { const mockAdapter = { exists: jest.fn(async (path: string) => path in files || Object.keys(files).some(k => k.startsWith(path + '/'))), @@ -36,10 +55,23 @@ function createMockAdapter(files: Record = {}): VaultFileAdapter } describe('SkillStorage', () => { + beforeEach(() => { + // Clear call counts and reset default implementations between tests + jest.clearAllMocks(); + // Default: global skills dir does not exist (isolates legacy tests) + fsMock.existsSync.mockReturnValue(false); + (fsMock.readdirSync as jest.Mock).mockReturnValue([]); + }); + it('exports SKILLS_PATH', () => { expect(SKILLS_PATH).toBe('.claude/skills'); }); + it('exports GLOBAL_SKILLS_DIR', () => { + expect(GLOBAL_SKILLS_DIR).toContain('.claude'); + expect(GLOBAL_SKILLS_DIR).toContain('skills'); + }); + describe('loadAll', () => { it('loads skills from subdirectories with SKILL.md', async () => { const adapter = createMockAdapter({ @@ -272,4 +304,103 @@ Prompt`, expect(adapter.deleteFolder).toHaveBeenCalledWith('.claude/skills/target'); }); }); + + describe('global skills', () => { + it('loads a global skill when the directory exists', async () => { + const adapter = createMockAdapter({}); + const skillContent = `--- +description: Global skill +--- +Global prompt`; + + fsMock.existsSync.mockImplementation((p) => { + const s = String(p); + return s === GLOBAL_SKILLS_DIR || s.endsWith('SKILL.md'); + }); + (fsMock.readdirSync as jest.Mock).mockReturnValue([ + makeDirent('my-global-skill', true), + ]); + fsMock.readFileSync.mockReturnValue(skillContent as any); + + const storage = new SkillStorage(adapter); + const skills = await storage.loadAll(); + + expect(skills).toHaveLength(1); + expect(skills[0].id).toBe('skill-global-my-global-skill'); + expect(skills[0].name).toBe('my-global-skill'); + expect(skills[0].description).toBe('Global skill'); + expect(skills[0].source).toBe('user'); + }); + + it('vault skill takes precedence over global skill with the same name', async () => { + const adapter = createMockAdapter({ + '.claude/skills/shared/SKILL.md': `--- +description: Vault version +--- +Vault prompt`, + }); + + fsMock.existsSync.mockImplementation((p) => { + const s = String(p); + return s === GLOBAL_SKILLS_DIR || s.endsWith('SKILL.md'); + }); + (fsMock.readdirSync as jest.Mock).mockReturnValue([ + makeDirent('shared', true), + ]); + fsMock.readFileSync.mockReturnValue(`--- +description: Global version +--- +Global prompt` as any); + + const storage = new SkillStorage(adapter); + const skills = await storage.loadAll(); + + expect(skills).toHaveLength(1); + expect(skills[0].id).toBe('skill-shared'); // vault id, not global + expect(skills[0].description).toBe('Vault version'); + }); + + it('does not fail when global skills directory does not exist', async () => { + const adapter = createMockAdapter({}); + fsMock.existsSync.mockReturnValue(false); + + const storage = new SkillStorage(adapter); + const skills = await storage.loadAll(); + + expect(skills).toEqual([]); + expect(fsMock.readdirSync).not.toHaveBeenCalled(); + }); + + it('skips a global subdir that has no SKILL.md', async () => { + const adapter = createMockAdapter({}); + + fsMock.existsSync.mockImplementation((p) => { + const s = String(p); + // Dir exists, but SKILL.md does not + return s === GLOBAL_SKILLS_DIR; + }); + (fsMock.readdirSync as jest.Mock).mockReturnValue([ + makeDirent('no-skill-here', true), + ]); + + const storage = new SkillStorage(adapter); + const skills = await storage.loadAll(); + + expect(skills).toHaveLength(0); + }); + + it('skips files (non-directories) inside global skills dir', async () => { + const adapter = createMockAdapter({}); + + fsMock.existsSync.mockReturnValue(true); + (fsMock.readdirSync as jest.Mock).mockReturnValue([ + makeDirent('some-file.md', false), + ]); + + const storage = new SkillStorage(adapter); + const skills = await storage.loadAll(); + + expect(skills).toHaveLength(0); + }); + }); }); From d4b71cbff54a7965a5c20bc5467a58c638ea2c0a Mon Sep 17 00:00:00 2001 From: execsumo <168054536+execsumo@users.noreply.github.com> Date: Fri, 27 Feb 2026 19:49:08 -0800 Subject: [PATCH 4/4] fix: ensure global skills load even when vault skills directory is missing --- src/core/storage/SkillStorage.ts | 44 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/core/storage/SkillStorage.ts b/src/core/storage/SkillStorage.ts index 14a0e3ae..75b56ebe 100644 --- a/src/core/storage/SkillStorage.ts +++ b/src/core/storage/SkillStorage.ts @@ -17,30 +17,32 @@ export class SkillStorage { const loadedNames = new Set(); try { - const folders = await this.adapter.listFolders(SKILLS_PATH); - - for (const folder of folders) { - const skillName = folder.split('/').pop()!; - const skillPath = `${SKILLS_PATH}/${skillName}/SKILL.md`; - - try { - if (!(await this.adapter.exists(skillPath))) continue; - - const content = await this.adapter.read(skillPath); - const parsed = parseSlashCommandContent(content); - - skills.push(parsedToSlashCommand(parsed, { - id: `skill-${skillName}`, - name: skillName, - source: 'user', - })); - loadedNames.add(skillName); - } catch { - // Non-critical: skip malformed skill files + if (await this.adapter.exists(SKILLS_PATH)) { + const folders = await this.adapter.listFolders(SKILLS_PATH); + + for (const folder of folders) { + const skillName = folder.split('/').pop()!; + const skillPath = `${SKILLS_PATH}/${skillName}/SKILL.md`; + + try { + if (!(await this.adapter.exists(skillPath))) continue; + + const content = await this.adapter.read(skillPath); + const parsed = parseSlashCommandContent(content); + + skills.push(parsedToSlashCommand(parsed, { + id: `skill-${skillName}`, + name: skillName, + source: 'user', + })); + loadedNames.add(skillName); + } catch { + // Non-critical: skip malformed skill files + } } } } catch { - return skills; + // Non-critical: skip vault skills if directory missing or inaccessible } // Also load user-level skills from ~/.claude/skills (global Claude Code skills).