mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-18 13:23:33 +08:00
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
This commit is contained in:
parent
f455065997
commit
6b77120515
@ -12,6 +12,7 @@ let projectRoot: string
|
||||
let originalConfigDir: string | undefined
|
||||
let connectSpy: ReturnType<typeof spyOn> | undefined
|
||||
let getClaudeCodeMcpConfigsSpy: ReturnType<typeof spyOn> | undefined
|
||||
let getAllMcpConfigsSpy: ReturnType<typeof spyOn> | undefined
|
||||
let reconnectSpy: ReturnType<typeof spyOn> | undefined
|
||||
let hostPreflightSpy: ReturnType<typeof spyOn> | 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,
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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')
|
||||
|
||||
@ -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<Response> {
|
||||
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]))
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -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<PluginId> {
|
||||
return new Set(
|
||||
Object.entries(getSettingsForSource(settingSource)?.enabledPlugins ?? {})
|
||||
.filter(([, v]) => v === true || Array.isArray(v))
|
||||
.filter(([, v]) => isEnabledPluginSettingValue(v))
|
||||
.map(([k]) => k),
|
||||
)
|
||||
}
|
||||
|
||||
@ -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,
|
||||
)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user