From 6b771205159f83e1efbc2291ad330520375a8c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Wed, 13 May 2026 22:11:24 +0800 Subject: [PATCH] Align settings resource discovery with runtime Settings reported false negatives for resources the runtime could load, because the API paths used narrower discovery rules than the execution path. This keeps the Settings surfaces read-only where appropriate while matching runtime visibility for linked skills, version-constrained plugins, and merged MCP sources. Constraint: Settings MCP list must not actively connect to servers while loading. Rejected: Add UI-specific fallbacks | would preserve drift between Settings and runtime loaders Confidence: high Scope-risk: moderate Directive: Keep Settings resource listing backed by the same shared loader semantics as runtime discovery. Tested: bun test src/server/__tests__/skills.test.ts Tested: bun test src/server/__tests__/plugins.test.ts Tested: bun test src/server/__tests__/mcp.test.ts Tested: bun run check:server Tested: bun run check:coverage Tested: bun run verify Not-tested: Live external MCP connector account state --- src/server/__tests__/mcp.test.ts | 34 +++++++++ src/server/__tests__/plugins.test.ts | 91 ++++++++++++++++++++++++ src/server/__tests__/skills.test.ts | 26 +++++++ src/server/api/mcp.ts | 5 +- src/server/api/skills.ts | 6 +- src/services/plugins/pluginOperations.ts | 7 +- src/utils/plugins/dependencyResolver.ts | 6 +- src/utils/plugins/pluginLoader.ts | 10 ++- 8 files changed, 175 insertions(+), 10 deletions(-) diff --git a/src/server/__tests__/mcp.test.ts b/src/server/__tests__/mcp.test.ts index c1650039..68b07185 100644 --- a/src/server/__tests__/mcp.test.ts +++ b/src/server/__tests__/mcp.test.ts @@ -12,6 +12,7 @@ let projectRoot: string let originalConfigDir: string | undefined let connectSpy: ReturnType | undefined let getClaudeCodeMcpConfigsSpy: ReturnType | undefined +let getAllMcpConfigsSpy: ReturnType | undefined let reconnectSpy: ReturnType | undefined let hostPreflightSpy: ReturnType | undefined @@ -77,6 +78,8 @@ describe('MCP API', () => { connectSpy = undefined getClaudeCodeMcpConfigsSpy?.mockRestore() getClaudeCodeMcpConfigsSpy = undefined + getAllMcpConfigsSpy?.mockRestore() + getAllMcpConfigsSpy = undefined reconnectSpy?.mockRestore() reconnectSpy = undefined hostPreflightSpy?.mockRestore() @@ -141,6 +144,37 @@ describe('MCP API', () => { expect(connectSpy).toHaveBeenCalled() }) + it('lists runtime-visible claude.ai MCP connectors as read-only settings entries', async () => { + getAllMcpConfigsSpy = spyOn(mcpConfig, 'getAllMcpConfigs').mockResolvedValue({ + servers: { + 'claude.ai Docs': { + type: 'claudeai-proxy', + url: 'https://mcp.example.com/docs', + id: 'srv_docs', + scope: 'claudeai', + }, + }, + errors: [], + }) + + const list = makeRequest('GET', `/api/mcp?cwd=${encodeURIComponent(projectRoot)}`) + const listRes = await handleMcpApi(list.req, list.url, list.segments) + + expect(listRes.status).toBe(200) + const listBody = await listRes.json() + + expect(listBody.servers).toContainEqual( + expect.objectContaining({ + name: 'claude.ai Docs', + scope: 'claudeai', + transport: 'claudeai-proxy', + canEdit: false, + canRemove: false, + }), + ) + expect(connectSpy).not.toHaveBeenCalled() + }) + it('rejects stdio MCP creation when the host command is unavailable', async () => { hostPreflightSpy?.mockResolvedValueOnce({ ok: false, diff --git a/src/server/__tests__/plugins.test.ts b/src/server/__tests__/plugins.test.ts index 48021ba7..8fbcf452 100644 --- a/src/server/__tests__/plugins.test.ts +++ b/src/server/__tests__/plugins.test.ts @@ -2,6 +2,10 @@ import { afterEach, beforeEach, describe, expect, it } from 'bun:test' import * as fs from 'node:fs/promises' import * as os from 'node:os' import * as path from 'node:path' +import { isEnabledPluginSettingValue } from '../../utils/plugins/dependencyResolver.js' +import { clearInstalledPluginsCache } from '../../utils/plugins/installedPluginsManager.js' +import { clearPluginCache, loadAllPluginsCacheOnly } from '../../utils/plugins/pluginLoader.js' +import { resetSettingsCache } from '../../utils/settings/settingsCache.js' import { handlePluginsApi } from '../api/plugins.js' let tmpDir: string @@ -31,9 +35,15 @@ describe('Plugins API', () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'claude-plugins-api-')) originalConfigDir = process.env.CLAUDE_CONFIG_DIR process.env.CLAUDE_CONFIG_DIR = tmpDir + clearInstalledPluginsCache() + clearPluginCache('plugins-api-test-setup') + resetSettingsCache() }) afterEach(async () => { + clearInstalledPluginsCache() + clearPluginCache('plugins-api-test-teardown') + resetSettingsCache() if (originalConfigDir === undefined) { delete process.env.CLAUDE_CONFIG_DIR } else { @@ -60,6 +70,87 @@ describe('Plugins API', () => { expect(body.summary.errorCount).toBe(0) }) + it('treats enabledPlugins version constraint arrays as enabled plugins', async () => { + const marketplaceRoot = path.join(tmpDir, 'marketplace-root') + const pluginRoot = path.join(marketplaceRoot, 'plugins', 'demo') + const pluginsDir = path.join(tmpDir, 'plugins') + const marketplaceFile = path.join(marketplaceRoot, '.claude-plugin', 'marketplace.json') + + await fs.mkdir(path.join(pluginRoot, '.claude-plugin'), { recursive: true }) + await fs.mkdir(path.dirname(marketplaceFile), { recursive: true }) + await fs.mkdir(pluginsDir, { recursive: true }) + + await fs.writeFile( + path.join(pluginRoot, '.claude-plugin', 'plugin.json'), + JSON.stringify({ + name: 'demo', + version: '1.0.0', + description: 'Demo plugin', + }), + 'utf-8', + ) + await fs.writeFile( + marketplaceFile, + JSON.stringify({ + name: 'test-market', + owner: { name: 'Test' }, + plugins: [ + { + name: 'demo', + source: './plugins/demo', + version: '1.0.0', + }, + ], + }), + 'utf-8', + ) + await fs.writeFile( + path.join(pluginsDir, 'known_marketplaces.json'), + JSON.stringify({ + 'test-market': { + source: { source: 'directory', path: marketplaceRoot }, + installLocation: marketplaceRoot, + lastUpdated: new Date(0).toISOString(), + }, + }), + 'utf-8', + ) + await fs.writeFile( + path.join(tmpDir, 'settings.json'), + JSON.stringify({ + enabledPlugins: { + 'demo@test-market': ['^1.0.0'], + }, + }), + 'utf-8', + ) + + expect(isEnabledPluginSettingValue(true)).toBe(true) + expect(isEnabledPluginSettingValue(['^1.0.0'])).toBe(true) + expect(isEnabledPluginSettingValue(false)).toBe(false) + expect(isEnabledPluginSettingValue(undefined)).toBe(false) + + const cacheOnlyResult = await loadAllPluginsCacheOnly() + expect(cacheOnlyResult.enabled).toContainEqual( + expect.objectContaining({ source: 'demo@test-market', enabled: true }), + ) + + clearPluginCache('plugins-api-test-full-load') + + const { req, url, segments } = makeRequest('GET', '/api/plugins') + const res = await handlePluginsApi(req, url, segments) + + expect(res.status).toBe(200) + const body = await res.json() as { + plugins: Array<{ id: string; enabled: boolean }> + summary: { enabled: number } + } + expect(body.plugins).toContainEqual( + expect.objectContaining({ id: 'demo@test-market', enabled: true }), + ) + expect(body.summary.enabled).toBe(1) + }) + it('POST /api/plugins/reload returns numeric counters', async () => { const { req, url, segments } = makeRequest('POST', '/api/plugins/reload', {}) const res = await handlePluginsApi(req, url, segments) diff --git a/src/server/__tests__/skills.test.ts b/src/server/__tests__/skills.test.ts index 9232124f..93ce2e5e 100644 --- a/src/server/__tests__/skills.test.ts +++ b/src/server/__tests__/skills.test.ts @@ -89,6 +89,32 @@ describe('Skills API', () => { expect(body.skills).toContainEqual(expect.objectContaining({ name: 'project-skill', source: 'project' })) }) + it('lists user skills installed through a directory symlink or junction', async () => { + const linkedSkillsRoot = path.join(tmpHome, '.agents', 'skills') + const userSkillsRoot = path.join(tmpHome, '.claude', 'skills') + const projectRoot = path.join(tmpHome, 'workspace') + const cwd = path.join(projectRoot, 'packages', 'app') + + await writeSkill( + linkedSkillsRoot, + 'linked-skill', + ['---', 'description: Linked skill', '---', '', '# Linked skill'].join('\n'), + ) + await fs.mkdir(userSkillsRoot, { recursive: true }) + await fs.symlink( + path.join(linkedSkillsRoot, 'linked-skill'), + path.join(userSkillsRoot, 'linked-skill'), + process.platform === 'win32' ? 'junction' : 'dir', + ) + + const { req, url, segments } = makeRequest(`/api/skills?cwd=${encodeURIComponent(cwd)}`) + const res = await handleSkillsApi(req, url, segments) + + expect(res.status).toBe(200) + const body = await res.json() as { skills: Array<{ name: string; source: string }> } + expect(body.skills).toContainEqual(expect.objectContaining({ name: 'linked-skill', source: 'user' })) + }) + it('resolves project skill details from the nearest project skills directory', async () => { const projectRoot = path.join(tmpHome, 'workspace') const nestedRoot = path.join(projectRoot, 'packages', 'app') diff --git a/src/server/api/mcp.ts b/src/server/api/mcp.ts index e5bc9545..802822a6 100644 --- a/src/server/api/mcp.ts +++ b/src/server/api/mcp.ts @@ -9,6 +9,7 @@ import { } from '../../services/mcp/client.js' import { addMcpConfig, + getAllMcpConfigs, getClaudeCodeMcpConfigs, getMcpConfigByName, isMcpServerDisabled, @@ -301,7 +302,7 @@ async function resolveServerForRuntimeAction( return configured } - const { servers } = await getClaudeCodeMcpConfigs() + const { servers } = await getAllMcpConfigs() return servers[name] ?? null } @@ -386,7 +387,7 @@ function cleanupSecureStorage(name: string, config: ScopedMcpServerConfig) { } async function listServers(): Promise { - const { servers } = await getClaudeCodeMcpConfigs() + const { servers } = await getAllMcpConfigs() const visibleServers = Object.entries(servers) .filter(([name, config]) => isVisibleServer(name, config)) .sort((a, b) => a[0].localeCompare(b[0])) diff --git a/src/server/api/skills.ts b/src/server/api/skills.ts index e76c2dc2..59233791 100644 --- a/src/server/api/skills.ts +++ b/src/server/api/skills.ts @@ -224,7 +224,11 @@ async function collectSkillsFromRoots( } for (const entry of entries) { - if (!entry.isDirectory() || entry.name.startsWith('.') || seenNames.has(entry.name)) { + if ( + (!entry.isDirectory() && !entry.isSymbolicLink()) || + entry.name.startsWith('.') || + seenNames.has(entry.name) + ) { continue } diff --git a/src/services/plugins/pluginOperations.ts b/src/services/plugins/pluginOperations.ts index 48e6b685..bd3c4c1c 100644 --- a/src/services/plugins/pluginOperations.ts +++ b/src/services/plugins/pluginOperations.ts @@ -25,6 +25,7 @@ import { import { findReverseDependents, formatReverseDependentsSuffix, + isEnabledPluginSettingValue, } from '../../utils/plugins/dependencyResolver.js' import { loadInstalledPluginsFromDisk, @@ -126,8 +127,8 @@ export function getProjectPathForScope(scope: PluginScope): string | undefined { * enablement active — the plugin keeps running. */ export function isPluginEnabledAtProjectScope(pluginId: string): boolean { - return ( - getSettingsForSource('projectSettings')?.enabledPlugins?.[pluginId] === true + return isEnabledPluginSettingValue( + getSettingsForSource('projectSettings')?.enabledPlugins?.[pluginId], ) } @@ -697,7 +698,7 @@ export async function setPluginEnabledOp( // `false` that masks the lower scope's `true`. const isCurrentlyEnabled = scope && !isOverride - ? scopeSettingsValue === true + ? isEnabledPluginSettingValue(scopeSettingsValue) : getPluginEditableScopes().has(pluginId) if (enabled === isCurrentlyEnabled) { return { diff --git a/src/utils/plugins/dependencyResolver.ts b/src/utils/plugins/dependencyResolver.ts index 5575c8c0..706e6bb9 100644 --- a/src/utils/plugins/dependencyResolver.ts +++ b/src/utils/plugins/dependencyResolver.ts @@ -24,6 +24,10 @@ import type { PluginId } from './schemas.js' */ const INLINE_MARKETPLACE = 'inline' +export function isEnabledPluginSettingValue(value: unknown): boolean { + return value === true || Array.isArray(value) +} + /** * Normalize a dependency reference to fully-qualified "name@marketplace" form. * Bare names (no @) inherit the marketplace of the plugin declaring them — @@ -277,7 +281,7 @@ export function getEnabledPluginIdsForScope( ): Set { return new Set( Object.entries(getSettingsForSource(settingSource)?.enabledPlugins ?? {}) - .filter(([, v]) => v === true || Array.isArray(v)) + .filter(([, v]) => isEnabledPluginSettingValue(v)) .map(([k]) => k), ) } diff --git a/src/utils/plugins/pluginLoader.ts b/src/utils/plugins/pluginLoader.ts index 0c861bf4..0c25de14 100644 --- a/src/utils/plugins/pluginLoader.ts +++ b/src/utils/plugins/pluginLoader.ts @@ -84,7 +84,10 @@ import type { HooksSettings } from '../settings/types.js' import { SettingsSchema } from '../settings/types.js' import { jsonParse, jsonStringify } from '../slowOperations.js' import { getAddDirEnabledPlugins } from './addDirPluginSettings.js' -import { verifyAndDemote } from './dependencyResolver.js' +import { + isEnabledPluginSettingValue, + verifyAndDemote, +} from './dependencyResolver.js' import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js' import { checkGitAvailable } from './gitAvailability.js' import { getInMemoryInstalledPlugins } from './installedPluginsManager.js' @@ -2049,12 +2052,13 @@ async function loadPluginsFromMarketplaces({ // (version for the full loader's first-pass probe, installPath for // the cache-only loader's direct read). const installEntry = installedPluginsData.plugins[pluginId]?.[0] + const enabled = isEnabledPluginSettingValue(enabledValue) return cacheOnly ? loadPluginFromMarketplaceEntryCacheOnly( result.entry, result.marketplaceInstallLocation, pluginId, - enabledValue === true, + enabled, errors, installEntry?.installPath, ) @@ -2062,7 +2066,7 @@ async function loadPluginsFromMarketplaces({ result.entry, result.marketplaceInstallLocation, pluginId, - enabledValue === true, + enabled, errors, installEntry?.version, )