diff --git a/frontend/src/api/generated.ts b/frontend/src/api/generated.ts index 2d1fb76..f18b42a 100644 --- a/frontend/src/api/generated.ts +++ b/frontend/src/api/generated.ts @@ -1093,6 +1093,8 @@ export interface components { configPath?: string | null; /** Env */ env?: components["schemas"]["McpEnvEntryResponse"][]; + /** Id */ + id: string; /** Label */ label: string; /** Logokey */ @@ -1103,6 +1105,11 @@ export interface components { payloadPreview: { [key: string]: unknown; }; + /** + * Recommended + * @default false + */ + recommended: boolean; /** * Sourcekind * @enum {string} @@ -1146,6 +1153,11 @@ export interface components { payloadPreview: { [key: string]: unknown; }; + /** + * Recommended + * @default false + */ + recommended: boolean; spec: components["schemas"]["McpServerSpecResponse"]; }; /** McpInstallConfigFieldResponse */ @@ -1215,10 +1227,14 @@ export interface components { }; /** McpInventoryEntryResponse */ McpInventoryEntryResponse: { - /** Availabilityreason */ + /** + * Availabilityreason + * @description Deprecated compatibility field; use mcpStatus.reason instead. + */ availabilityReason?: string | null; /** * Availabilitystatus + * @description Deprecated compatibility field; use mcpStatus instead. * @enum {string} */ availabilityStatus: "available" | "unavailable"; @@ -1458,10 +1474,14 @@ export interface components { }; /** McpServerDetailResponse */ McpServerDetailResponse: { - /** Availabilityreason */ + /** + * Availabilityreason + * @description Deprecated compatibility field; use mcpStatus.reason instead. + */ availabilityReason?: string | null; /** * Availabilitystatus + * @description Deprecated compatibility field; use mcpStatus instead. * @enum {string} */ availabilityStatus: "available" | "unavailable"; diff --git a/frontend/src/api/openapi.json b/frontend/src/api/openapi.json index 9743c5e..2697179 100644 --- a/frontend/src/api/openapi.json +++ b/frontend/src/api/openapi.json @@ -900,6 +900,10 @@ "title": "Env", "type": "array" }, + "id": { + "title": "Id", + "type": "string" + }, "label": { "title": "Label", "type": "string" @@ -931,6 +935,11 @@ "title": "Payloadpreview", "type": "object" }, + "recommended": { + "default": false, + "title": "Recommended", + "type": "boolean" + }, "sourceKind": { "enum": [ "managed", @@ -944,6 +953,7 @@ } }, "required": [ + "id", "sourceKind", "label", "payloadPreview", @@ -1071,6 +1081,11 @@ "title": "Payloadpreview", "type": "object" }, + "recommended": { + "default": false, + "title": "Recommended", + "type": "boolean" + }, "spec": { "$ref": "#/components/schemas/McpServerSpecResponse" } @@ -1281,9 +1296,11 @@ "type": "null" } ], + "description": "Deprecated compatibility field; use mcpStatus.reason instead.", "title": "Availabilityreason" }, "availabilityStatus": { + "description": "Deprecated compatibility field; use mcpStatus instead.", "enum": [ "available", "unavailable" @@ -2114,9 +2131,11 @@ "type": "null" } ], + "description": "Deprecated compatibility field; use mcpStatus.reason instead.", "title": "Availabilityreason" }, "availabilityStatus": { + "description": "Deprecated compatibility field; use mcpStatus instead.", "enum": [ "available", "unavailable" diff --git a/frontend/src/features/mcp/components/detail/McpNeedsReviewDetailView.test.tsx b/frontend/src/features/mcp/components/detail/McpNeedsReviewDetailView.test.tsx index 000f8ec..e27eeb7 100644 --- a/frontend/src/features/mcp/components/detail/McpNeedsReviewDetailView.test.tsx +++ b/frontend/src/features/mcp/components/detail/McpNeedsReviewDetailView.test.tsx @@ -22,6 +22,7 @@ function makeGroup(overrides: Partial = {}): McpIdentityGro sightings: [ { harness: "cursor", + recommended: true, label: "Cursor", logoKey: "cursor", configPath: "/c/.cursor/mcp.json", @@ -40,6 +41,7 @@ function makeGroup(overrides: Partial = {}): McpIdentityGro }, { harness: "claude", + recommended: false, label: "Claude", logoKey: "claude", configPath: "/c/.claude.json", diff --git a/frontend/src/features/mcp/components/detail/McpServerDetailView.test.tsx b/frontend/src/features/mcp/components/detail/McpServerDetailView.test.tsx index 1a0a759..81b74c6 100644 --- a/frontend/src/features/mcp/components/detail/McpServerDetailView.test.tsx +++ b/frontend/src/features/mcp/components/detail/McpServerDetailView.test.tsx @@ -349,8 +349,10 @@ describe("McpServerDetailView", () => { ], configChoices: [ { + id: "managed", sourceKind: "managed", observedHarness: null, + recommended: true, label: "Skill Manager config", logoKey: null, configPath: null, @@ -519,8 +521,10 @@ describe("McpServerDetailView", () => { ], configChoices: [ { + id: "managed", sourceKind: "managed", observedHarness: null, + recommended: false, label: "Skill Manager config", logoKey: null, configPath: null, @@ -537,8 +541,10 @@ describe("McpServerDetailView", () => { env: [], }, { + id: "harness:cursor", sourceKind: "harness", observedHarness: "cursor", + recommended: true, label: "Cursor config", logoKey: "cursor", configPath: "/tmp/.cursor/mcp.json", diff --git a/frontend/src/features/mcp/components/detail/McpServerDetailView.tsx b/frontend/src/features/mcp/components/detail/McpServerDetailView.tsx index 423e80c..8d7b106 100644 --- a/frontend/src/features/mcp/components/detail/McpServerDetailView.tsx +++ b/frontend/src/features/mcp/components/detail/McpServerDetailView.tsx @@ -17,7 +17,7 @@ import { formatDisplayHeaders } from "../../model/display-secrets"; import type { McpInstallConfigValues } from "../../model/install-config"; import { mcpStatusReason } from "../../model/mcp-status"; import { mcpServerSourceLinks, resolveMcpRegistryName } from "../../model/mcp-source-links"; -import { useMcpEnableConfigGate } from "../../model/use-mcp-enable-config-gate"; +import { useMcpEnableWorkflow } from "../../model/use-mcp-enable-workflow"; import { McpInstallConfigDialog } from "../config/McpInstallConfigDialog"; import { McpConfigChoiceDialog, @@ -74,7 +74,7 @@ export function McpServerDetailView({ cancelConfig: cancelEnableConfig, submitConfig: submitEnableConfig, configError: enableConfigError, - } = useMcpEnableConfigGate({ + } = useMcpEnableWorkflow({ loadErrorMessage: copy.detail.unableToLoadInstallConfig, }); @@ -221,20 +221,15 @@ export function McpServerDetailView({ canEnable={detail.canEnable} serverPending={isServerPending} pendingPerHarness={pendingPerHarness} - onEnable={(harness) => - requestEnable({ - spec, - displayName, - targetLabel: harness, - installConfigStatus: detail.installConfigStatus, - onProceed: (config) => { - if (config === undefined) { - onEnableHarness(harness); - return; - } - onEnableHarness(harness, config); - }, - })} + onEnable={(harness) => { + requestEnable(detail, harness, (config) => { + if (config === undefined) { + onEnableHarness(harness); + return; + } + onEnableHarness(harness, config); + }); + }} onDisable={onDisableHarness} onResolveConfigClick={() => setResolveDialogOpen(true)} canResolveConfig={canResolveConfig} @@ -296,7 +291,7 @@ export function McpServerDetailView({ function configChoiceToOption(choice: McpConfigChoiceDto, copy: McpCopy): McpConfigChoiceOption { return { - id: choice.sourceKind === "managed" ? "managed" : (choice.observedHarness ?? choice.label), + id: choice.id, sourceKind: choice.sourceKind, observedHarness: choice.observedHarness, label: choice.sourceKind === "managed" ? copy.detail.skillManagerConfig : choice.label, @@ -305,6 +300,7 @@ function configChoiceToOption(choice: McpConfigChoiceDto, copy: McpCopy): McpCon payloadPreview: choice.payloadPreview, spec: choice.spec, env: choice.env ?? [], + recommended: choice.recommended, }; } diff --git a/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.test.tsx b/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.test.tsx index ec8817e..6e4d656 100644 --- a/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.test.tsx +++ b/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.test.tsx @@ -14,6 +14,7 @@ function options(): McpConfigChoiceOption[] { id: "cursor", sourceKind: "harness", observedHarness: "cursor", + recommended: true, label: "Cursor", logoKey: "cursor", configPath: "/c/.cursor/mcp.json", @@ -34,6 +35,7 @@ function options(): McpConfigChoiceOption[] { id: "claude", sourceKind: "harness", observedHarness: "claude", + recommended: false, label: "Claude", logoKey: "claude", configPath: "/c/.claude.json", diff --git a/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.tsx b/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.tsx index 37833ed..7cf04b9 100644 --- a/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.tsx +++ b/frontend/src/features/mcp/components/edit/McpConfigChoiceDialog.tsx @@ -4,7 +4,6 @@ import { AlertTriangle, Check, ChevronDown, ChevronRight, Loader2, X } from "luc import { UiTooltip } from "../../../../components/ui/UiTooltip"; import type { - McpConfigChoiceDto, McpEnvEntryDto, McpServerSpecDto, } from "../../api/management-types"; @@ -13,7 +12,6 @@ import { maskMcpPayloadPreview } from "../../model/display-secrets"; import { envChipLabel, formatEnvKeyPreview, - pickRecommendedConfigChoice, summarizeMcpConfig, } from "../../model/selectors"; @@ -27,6 +25,7 @@ export interface McpConfigChoiceOption { payloadPreview: Record; spec: McpServerSpecDto; env: McpEnvEntryDto[]; + recommended?: boolean; } interface McpConfigChoiceDialogProps { @@ -50,7 +49,7 @@ export function McpConfigChoiceDialog({ }: McpConfigChoiceDialogProps) { const copy = useMcpCopy(); const recommendedId = useMemo( - () => pickRecommendedConfigChoice(options.map(toConfigChoiceDto)), + () => options.find((option) => option.recommended)?.id ?? null, [options], ); const [chosenId, setChosenId] = useState( @@ -224,16 +223,3 @@ export function McpConfigChoiceDialog({ ); } - -function toConfigChoiceDto(option: McpConfigChoiceOption): McpConfigChoiceDto { - return { - sourceKind: option.sourceKind, - observedHarness: option.observedHarness ?? null, - label: option.label, - logoKey: option.logoKey ?? null, - configPath: option.configPath ?? null, - payloadPreview: option.payloadPreview, - spec: option.spec, - env: option.env, - }; -} diff --git a/frontend/src/features/mcp/model/selectors.test.ts b/frontend/src/features/mcp/model/selectors.test.ts index 0e4aaa3..47b6247 100644 --- a/frontend/src/features/mcp/model/selectors.test.ts +++ b/frontend/src/features/mcp/model/selectors.test.ts @@ -10,7 +10,6 @@ import { filterMcpServersInUse, formatEnvKeyPreview, isMcpHarnessAddressable, - pickRecommendedSighting, pillCounts, summarizeSighting, urlHasEmbeddedCredential, @@ -169,6 +168,7 @@ function makeSighting( ): McpIdentitySightingDto { return { harness, + recommended: false, label: harness.charAt(0).toUpperCase() + harness.slice(1), logoKey: harness, configPath: `/mock/${harness}.json`, @@ -265,56 +265,6 @@ describe("summarizeSighting", () => { }); }); -describe("pickRecommendedSighting", () => { - it("returns null for empty input", () => { - expect(pickRecommendedSighting([])).toBeNull(); - }); - - it("prefers stdio with env-var refs over stdio with literal", () => { - const envRef = makeSighting( - "cursor", - { transport: "stdio", command: "npx" }, - [{ key: "K", value: "${env:K}", isEnvRef: true }], - ); - const envLiteral = makeSighting( - "codex", - { transport: "stdio", command: "npx" }, - [{ key: "K", value: "literal", isEnvRef: false }], - ); - expect(pickRecommendedSighting([envLiteral, envRef])).toBe("cursor"); - }); - - it("prefers any stdio over http", () => { - const stdio = makeSighting("codex", { transport: "stdio", command: "npx" }); - const http = makeSighting("claude", { - transport: "http", - url: "https://mcp.example.com", - }); - expect(pickRecommendedSighting([http, stdio])).toBe("codex"); - }); - - it("prefers http without URL credential over http with URL credential", () => { - const dirty = makeSighting("claude", { - transport: "http", - url: "https://a.example?api_key=xyz", - }); - const clean = makeSighting("cursor", { transport: "http", url: "https://b.example" }); - expect(pickRecommendedSighting([dirty, clean])).toBe("cursor"); - }); - - it("returns null when all options embed credentials in URL", () => { - const a = makeSighting("claude", { - transport: "http", - url: "https://a.example?api_key=xyz", - }); - const b = makeSighting("cursor", { - transport: "http", - url: "https://b.example?token=xyz", - }); - expect(pickRecommendedSighting([a, b])).toBeNull(); - }); -}); - describe("isMcpHarnessAddressable", () => { it("returns true when the CLI is installed", () => { expect( diff --git a/frontend/src/features/mcp/model/selectors.ts b/frontend/src/features/mcp/model/selectors.ts index 651886f..3366109 100644 --- a/frontend/src/features/mcp/model/selectors.ts +++ b/frontend/src/features/mcp/model/selectors.ts @@ -1,6 +1,5 @@ import type { McpBindingDto, - McpConfigChoiceDto, McpEnvEntryDto, McpIdentitySightingDto, McpInventoryColumnDto, @@ -269,45 +268,3 @@ export function formatEnvKeyPreview(keys: readonly string[]): string { export function envChipLabel(count: number): string { return count === 1 ? "1 env var" : `${count} env vars`; } - -/** - * Suggest a canonical harness when payloads differ. - * Priority: - * 1) stdio with env-var references (safest — no literal secrets anywhere) - * 2) stdio (literal env is still safer than URL-embedded) - * 3) remote without credential embedded in URL - * 4) none — caller should fall back to the first sighting - */ -export function pickRecommendedSighting( - sightings: readonly McpIdentitySightingDto[], -): string | null { - if (sightings.length === 0) return null; - const hasEnvRef = (s: McpIdentitySightingDto) => (s.env ?? []).some((e) => e.isEnvRef); - - const tier1 = sightings.find((s) => s.spec.transport === "stdio" && hasEnvRef(s)); - if (tier1) return tier1.harness; - const tier2 = sightings.find((s) => s.spec.transport === "stdio"); - if (tier2) return tier2.harness; - const tier3 = sightings.find( - (s) => s.spec.transport !== "stdio" && !urlHasEmbeddedCredential(s.spec.url), - ); - if (tier3) return tier3.harness; - return null; -} - -export function pickRecommendedConfigChoice( - choices: readonly McpConfigChoiceDto[], -): string | null { - const harnessChoices = choices.filter((choice) => choice.sourceKind === "harness"); - if (harnessChoices.length === 0) return choices[0]?.sourceKind === "managed" ? "managed" : null; - const hasEnvRef = (choice: McpConfigChoiceDto) => (choice.env ?? []).some((e) => e.isEnvRef); - const tier1 = harnessChoices.find((choice) => choice.spec.transport === "stdio" && hasEnvRef(choice)); - if (tier1?.observedHarness) return tier1.observedHarness; - const tier2 = harnessChoices.find((choice) => choice.spec.transport === "stdio"); - if (tier2?.observedHarness) return tier2.observedHarness; - const tier3 = harnessChoices.find( - (choice) => choice.spec.transport !== "stdio" && !urlHasEmbeddedCredential(choice.spec.url), - ); - if (tier3?.observedHarness) return tier3.observedHarness; - return choices[0]?.sourceKind === "managed" ? "managed" : (harnessChoices[0]?.observedHarness ?? null); -} diff --git a/frontend/src/features/mcp/model/use-mcp-enable-workflow.ts b/frontend/src/features/mcp/model/use-mcp-enable-workflow.ts new file mode 100644 index 0000000..4917b1d --- /dev/null +++ b/frontend/src/features/mcp/model/use-mcp-enable-workflow.ts @@ -0,0 +1,70 @@ +import { useCallback } from "react"; + +import type { McpInventoryEntryDto, McpServerDetailDto } from "../api/management-types"; +import type { McpInstallConfigValues } from "./install-config"; +import { useMcpEnableConfigGate } from "./use-mcp-enable-config-gate"; + +type EnableEntry = Pick< + McpInventoryEntryDto | McpServerDetailDto, + "displayName" | "installConfigStatus" | "spec" +>; + +interface UseMcpEnableWorkflowParams { + loadErrorMessage: string; + bulkRequiresSingleMessage?: (displayName: string) => string; +} + +export function useMcpEnableWorkflow({ + loadErrorMessage, + bulkRequiresSingleMessage, +}: UseMcpEnableWorkflowParams) { + const configGate = useMcpEnableConfigGate({ loadErrorMessage }); + + const requestEnable = useCallback( + ( + entry: EnableEntry, + targetLabel: string, + onProceed: (config?: McpInstallConfigValues) => void, + ): void => { + configGate.requestEnable({ + spec: entry.spec ?? null, + displayName: entry.displayName, + targetLabel, + installConfigStatus: entry.installConfigStatus, + onProceed, + }); + }, + [configGate], + ); + + const requestBulkEnable = useCallback( + async ( + entries: readonly McpInventoryEntryDto[], + onSingleEntry: (entry: McpInventoryEntryDto) => void, + onManyEntries: () => Promise, + onBlocked: (message: string) => void, + ): Promise => { + if (entries.length === 0) return; + if (entries.length === 1) { + onSingleEntry(entries[0]); + return; + } + const blocked = entries.find((entry) => entry.installConfigStatus.missingRequired.length > 0); + if (blocked) { + onBlocked(bulkRequiresSingleMessage?.(blocked.displayName) ?? blocked.displayName); + return; + } + await onManyEntries(); + }, + [bulkRequiresSingleMessage], + ); + + return { + requestEnable, + requestBulkEnable, + pendingConfig: configGate.pendingConfig, + cancelConfig: configGate.cancelConfig, + submitConfig: configGate.submitConfig, + configError: configGate.configError, + }; +} diff --git a/frontend/src/features/mcp/model/use-mcp-management-controller.ts b/frontend/src/features/mcp/model/use-mcp-management-controller.ts index 9f0b22a..29f8eba 100644 --- a/frontend/src/features/mcp/model/use-mcp-management-controller.ts +++ b/frontend/src/features/mcp/model/use-mcp-management-controller.ts @@ -59,9 +59,7 @@ export function useMcpManagementController() { for (const entry of inventory.entries) { if ( entry.kind !== "managed" || - entry.mcpStatus.kind === "needs_config" || - entry.availabilityStatus !== "unavailable" || - entry.availabilityReason !== null || + entry.mcpStatus.kind !== "unchecked" || !entry.spec ) { continue; diff --git a/frontend/src/features/mcp/screens/McpInUsePage.tsx b/frontend/src/features/mcp/screens/McpInUsePage.tsx index a427695..483e780 100644 --- a/frontend/src/features/mcp/screens/McpInUsePage.tsx +++ b/frontend/src/features/mcp/screens/McpInUsePage.tsx @@ -23,7 +23,7 @@ import { pillCounts, type InUsePillValue, } from "../model/selectors"; -import { useMcpEnableConfigGate } from "../model/use-mcp-enable-config-gate"; +import { useMcpEnableWorkflow } from "../model/use-mcp-enable-workflow"; import { useMcpManagementController } from "../model/use-mcp-management-controller"; import { useMcpInUseViewMode, type McpInUseViewMode } from "../model/useMcpInUseViewMode"; @@ -65,12 +65,14 @@ export default function McpInUsePage() { const common = useCommonCopy(); const { requestEnable, + requestBulkEnable, pendingConfig: pendingEnableConfig, cancelConfig: cancelEnableConfig, submitConfig: submitEnableConfig, configError: enableConfigError, - } = useMcpEnableConfigGate({ + } = useMcpEnableWorkflow({ loadErrorMessage: copy.detail.unableToLoadInstallConfig, + bulkRequiresSingleMessage: copy.detail.installConfig.bulkRequiresSingle, }); const viewModeOptions: readonly ViewModeOption[] = useMemo( () => [ @@ -113,13 +115,7 @@ export default function McpInUsePage() { ): void => { const entry = findEntry(name); if (!entry) return; - requestEnable({ - spec: entry.spec ?? null, - displayName: entry.displayName, - targetLabel, - installConfigStatus: entry.installConfigStatus, - onProceed, - }); + requestEnable(entry, targetLabel, onProceed); }, [findEntry, requestEnable], ); @@ -152,28 +148,24 @@ export default function McpInUsePage() { const handleBulkEnableAll = useCallback(async (): Promise => { const selectedNames = Array.from(multiSelectedNames); - if (selectedNames.length === 1) { - const [name] = selectedNames; - handleCardSetHarnesses(name, "enabled"); - return; - } - const blocked = selectedNames + const selectedEntries = selectedNames .map((name) => findEntry(name)) - .find((entry): entry is McpInventoryEntryDto => - Boolean(entry?.installConfigStatus.missingRequired.length), - ) ?? null; - if (blocked) { - setPageActionErrorMessage(copy.detail.installConfig.bulkRequiresSingle(blocked.displayName)); - return; - } - setPageActionErrorMessage(""); - await handleMultiSelectEnableAll(); + .filter((entry): entry is McpInventoryEntryDto => Boolean(entry)); + await requestBulkEnable( + selectedEntries, + (entry) => handleCardSetHarnesses(entry.name, "enabled"), + async () => { + setPageActionErrorMessage(""); + await handleMultiSelectEnableAll(); + }, + setPageActionErrorMessage, + ); }, [ - copy.detail.installConfig, findEntry, handleCardSetHarnesses, handleMultiSelectEnableAll, multiSelectedNames, + requestBulkEnable, ]); const dismissVisibleActionError = useCallback(() => { diff --git a/frontend/src/features/mcp/screens/McpNeedsReviewPage.tsx b/frontend/src/features/mcp/screens/McpNeedsReviewPage.tsx index 0f88468..e183a7a 100644 --- a/frontend/src/features/mcp/screens/McpNeedsReviewPage.tsx +++ b/frontend/src/features/mcp/screens/McpNeedsReviewPage.tsx @@ -198,7 +198,7 @@ export default function McpNeedsReviewPage() { function optionsForGroup(group: McpIdentityGroupDto): McpConfigChoiceOption[] { return group.sightings.map((sighting) => ({ - id: sighting.harness, + id: `harness:${sighting.harness}`, sourceKind: "harness", observedHarness: sighting.harness, label: sighting.label, @@ -207,5 +207,6 @@ function optionsForGroup(group: McpIdentityGroupDto): McpConfigChoiceOption[] { payloadPreview: sighting.payloadPreview, spec: sighting.spec, env: sighting.env ?? [], + recommended: sighting.recommended, })); } diff --git a/skill_manager/api/schemas/mcp.py b/skill_manager/api/schemas/mcp.py index da892a1..971e073 100644 --- a/skill_manager/api/schemas/mcp.py +++ b/skill_manager/api/schemas/mcp.py @@ -113,8 +113,14 @@ class McpInventoryEntryResponse(BaseModel): spec: McpServerSpecResponse | None = None canEnable: bool enabledStatus: Literal["enabled", "disabled"] - availabilityStatus: Literal["available", "unavailable"] - availabilityReason: str | None = None + availabilityStatus: Literal["available", "unavailable"] = Field( + ..., + description="Deprecated compatibility field; use mcpStatus instead.", + ) + availabilityReason: str | None = Field( + default=None, + description="Deprecated compatibility field; use mcpStatus.reason instead.", + ) mcpStatus: McpStatusResponse installConfigStatus: McpInstallConfigStatusResponse sightings: list[McpBindingResponse] @@ -163,6 +169,7 @@ class McpEnvEntryResponse(BaseModel): class McpConfigChoiceResponse(BaseModel): + id: str sourceKind: Literal["managed", "harness"] observedHarness: str | None = Field(default=None, title="Observed harness") label: str @@ -171,6 +178,7 @@ class McpConfigChoiceResponse(BaseModel): payloadPreview: dict[str, object] spec: McpServerSpecResponse env: list[McpEnvEntryResponse] = Field(default_factory=list) + recommended: bool = False class McpMarketplaceLinkResponse(BaseModel): @@ -210,6 +218,7 @@ class McpIdentitySightingResponse(BaseModel): payloadPreview: dict[str, object] spec: McpServerSpecResponse env: list[McpEnvEntryResponse] = Field(default_factory=list) + recommended: bool = False class McpAdoptionIssueResponse(BaseModel): diff --git a/skill_manager/application/mcp/config_choice.py b/skill_manager/application/mcp/config_choice.py new file mode 100644 index 0000000..aa7cfa8 --- /dev/null +++ b/skill_manager/application/mcp/config_choice.py @@ -0,0 +1,182 @@ +from __future__ import annotations + +from dataclasses import dataclass + +from skill_manager.errors import MutationError + +from .contracts import McpHarnessScan +from .env import is_env_var_reference +from .redaction import annotate_redacted_env, redact_payload, redacted_spec_dict +from .store import McpServerSpec + + +@dataclass(frozen=True) +class McpConfigChoice: + id: str + source_kind: str + observed_harness: str | None + label: str + logo_key: str | None + config_path: str | None + payload_preview: dict[str, object] + spec: McpServerSpec + env: list[dict[str, object]] + recommended: bool = False + + def to_dict(self) -> dict[str, object]: + return { + "id": self.id, + "sourceKind": self.source_kind, + "observedHarness": self.observed_harness, + "label": self.label, + "logoKey": self.logo_key, + "configPath": self.config_path, + "payloadPreview": self.payload_preview, + "spec": redacted_spec_dict(self.spec), + "env": self.env, + "recommended": self.recommended, + } + + +def config_choices_payload( + name: str, + managed_spec: McpServerSpec, + scans: tuple[McpHarnessScan, ...], +) -> list[dict[str, object]]: + choices = _config_choices(name, managed_spec, scans) + recommended_id = _recommended_choice_id(choices) + return [ + ( + choice + if choice.id != recommended_id + else McpConfigChoice( + id=choice.id, + source_kind=choice.source_kind, + observed_harness=choice.observed_harness, + label=choice.label, + logo_key=choice.logo_key, + config_path=choice.config_path, + payload_preview=choice.payload_preview, + spec=choice.spec, + env=choice.env, + recommended=True, + ) + ).to_dict() + for choice in choices + ] + + +def observed_spec_from_scans( + name: str, + observed_harness: str, + scans: tuple[McpHarnessScan, ...], +) -> McpServerSpec: + for scan in scans: + if scan.harness != observed_harness: + continue + for entry in scan.entries: + if entry.name != name: + continue + if entry.parsed_spec is None: + raise MutationError( + entry.parse_issue or f"unable to parse '{name}' in {observed_harness}", + status=409, + ) + return entry.parsed_spec + raise MutationError(f"server '{name}' was not observed in harness '{observed_harness}'", status=404) + + +def recommended_observed_harness(sightings) -> str | None: + sightings = list(sightings) + if not sightings: + return None + + for sighting in sightings: + if sighting.spec.transport == "stdio" and _spec_has_env_ref(sighting.spec): + return sighting.harness + for sighting in sightings: + if sighting.spec.transport == "stdio": + return sighting.harness + for sighting in sightings: + if sighting.spec.transport != "stdio" and not _url_has_embedded_credential(sighting.spec.url): + return sighting.harness + return None + + +def _config_choices( + name: str, + managed_spec: McpServerSpec, + scans: tuple[McpHarnessScan, ...], +) -> list[McpConfigChoice]: + choices: list[McpConfigChoice] = [ + McpConfigChoice( + id="managed", + source_kind="managed", + observed_harness=None, + label="Managed config", + logo_key=None, + config_path=None, + payload_preview=redacted_spec_dict(managed_spec), + spec=managed_spec, + env=annotate_redacted_env(managed_spec.env), + ) + ] + for scan in scans: + for observed in scan.entries: + if observed.name != name or observed.state != "drifted": + continue + if observed.parsed_spec is None: + continue + choices.append( + McpConfigChoice( + id=f"harness:{scan.harness}", + source_kind="harness", + observed_harness=scan.harness, + label=f"{scan.label} config", + logo_key=scan.logo_key, + config_path=str(scan.config_path) if scan.config_present else None, + payload_preview=redact_payload(dict(observed.raw_payload or {})), + spec=observed.parsed_spec, + env=annotate_redacted_env(observed.parsed_spec.env), + ) + ) + return choices + + +def _recommended_choice_id(choices: list[McpConfigChoice]) -> str | None: + harness_choices = [choice for choice in choices if choice.source_kind == "harness"] + if not harness_choices: + return "managed" if choices else None + + def has_env_ref(choice: McpConfigChoice) -> bool: + return any(bool(entry.get("isEnvRef")) for entry in choice.env) + + for choice in harness_choices: + if choice.spec.transport == "stdio" and has_env_ref(choice): + return choice.id + for choice in harness_choices: + if choice.spec.transport == "stdio": + return choice.id + for choice in harness_choices: + if choice.spec.transport != "stdio" and not _url_has_embedded_credential(choice.spec.url): + return choice.id + return choices[0].id if choices else None + + +def _url_has_embedded_credential(url: str | None) -> bool: + if not url: + return False + lowered = url.lower() + return any(token in lowered for token in ("api_key=", "api-key=", "token=", "secret=", "auth=", "authorization=")) + + +def _spec_has_env_ref(spec: McpServerSpec) -> bool: + return any(is_env_var_reference(value) for _key, value in spec.env or ()) + + +__all__ = [ + "McpConfigChoice", + "config_choices_payload", + "observed_spec_from_scans", + "recommended_observed_harness", +] diff --git a/skill_manager/application/mcp/harness_application.py b/skill_manager/application/mcp/harness_application.py new file mode 100644 index 0000000..8891fb1 --- /dev/null +++ b/skill_manager/application/mcp/harness_application.py @@ -0,0 +1,126 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Callable, Iterable, Literal + +from .contracts import McpHarnessAdapter +from .read_models import McpReadModelService +from .store import McpServerSpec + + +HarnessAction = Literal["enable", "disable"] +ManifestCommit = Callable[[], None] + + +@dataclass(frozen=True) +class McpHarnessApplicationResult: + succeeded: list[str] + failed: list[dict[str, str]] + + @property + def ok(self) -> bool: + return not self.failed + + @property + def changed(self) -> bool: + return bool(self.succeeded) + + def to_dict(self) -> dict[str, object]: + return { + "ok": self.ok, + "succeeded": self.succeeded, + "failed": self.failed, + } + + +class McpHarnessApplication: + def __init__(self, read_models: McpReadModelService) -> None: + self.read_models = read_models + + def enable_one( + self, + adapter: McpHarnessAdapter, + spec: McpServerSpec, + *, + commit: ManifestCommit | None = None, + ) -> McpHarnessApplicationResult: + try: + adapter.enable_server(spec) + except Exception as error: # noqa: BLE001 + return McpHarnessApplicationResult( + succeeded=[], + failed=[{"harness": adapter.harness, "error": str(error)}], + ) + if commit is not None: + commit() + self.read_models.invalidate() + return McpHarnessApplicationResult(succeeded=[adapter.harness], failed=[]) + + def enable_many( + self, + spec: McpServerSpec, + harnesses: Iterable[str], + *, + writable_only: bool = False, + skip_harnesses: Iterable[str] = (), + commit: ManifestCommit | None = None, + ) -> McpHarnessApplicationResult: + targets = set(harnesses) + skipped = set(skip_harnesses) + adapters = ( + self.read_models.enabled_writable_adapters() + if writable_only + else self.read_models.enabled_adapters() + ) + succeeded: list[str] = [] + failed: list[dict[str, str]] = [] + for adapter in adapters: + if adapter.harness not in targets or adapter.harness in skipped: + continue + try: + adapter.enable_server(spec) + except Exception as error: # noqa: BLE001 + failed.append({"harness": adapter.harness, "error": str(error)}) + continue + succeeded.append(adapter.harness) + + if succeeded: + if commit is not None: + commit() + self.read_models.invalidate() + return McpHarnessApplicationResult(succeeded=succeeded, failed=failed) + + def disable_many( + self, + name: str, + harnesses: Iterable[str], + *, + addressable_only: bool = False, + remove_after_full_success: Callable[[], None] | None = None, + ) -> McpHarnessApplicationResult: + targets = set(harnesses) + adapters = ( + self.read_models.enabled_addressable_adapters() + if addressable_only + else self.read_models.enabled_adapters() + ) + succeeded: list[str] = [] + failed: list[dict[str, str]] = [] + for adapter in adapters: + if adapter.harness not in targets: + continue + try: + adapter.disable_server(name) + except Exception as error: # noqa: BLE001 + failed.append({"harness": adapter.harness, "error": str(error)}) + continue + succeeded.append(adapter.harness) + + if not failed and remove_after_full_success is not None: + remove_after_full_success() + if succeeded or (not failed and remove_after_full_success is not None): + self.read_models.invalidate() + return McpHarnessApplicationResult(succeeded=succeeded, failed=failed) + + +__all__ = ["McpHarnessApplication", "McpHarnessApplicationResult"] diff --git a/skill_manager/application/mcp/install_intent.py b/skill_manager/application/mcp/install_intent.py new file mode 100644 index 0000000..180d6a8 --- /dev/null +++ b/skill_manager/application/mcp/install_intent.py @@ -0,0 +1,219 @@ +from __future__ import annotations + +from dataclasses import dataclass, replace +from typing import Literal, Mapping + +from skill_manager.errors import MutationError + +from .install_config import resolved_config_values +from .install_resolver import ( + RegistryInstallOption, + registry_install_option, + registry_install_option_by_key, + resolve_registry_server_spec, +) +from .store import McpServerSpec + + +@dataclass(frozen=True) +class RegistryInstallIntent: + kind: Literal["registry"] + qualified_name: str + option_key: str + values: tuple[tuple[str, str], ...] = () + + @classmethod + def create( + cls, + *, + qualified_name: str, + option_key: str, + values: Mapping[str, str] | None = None, + ) -> "RegistryInstallIntent": + return cls( + kind="registry", + qualified_name=qualified_name, + option_key=option_key, + values=tuple(sorted((values or {}).items())), + ) + + @classmethod + def from_dict(cls, payload: Mapping[str, object]) -> "RegistryInstallIntent | None": + if payload.get("kind") != "registry": + return None + qualified_name = payload.get("qualifiedName") + option_key = payload.get("optionKey") + if not isinstance(qualified_name, str) or not isinstance(option_key, str): + return None + raw_values = payload.get("values") + values = ( + tuple((str(key), str(value)) for key, value in raw_values.items()) + if isinstance(raw_values, Mapping) + else () + ) + return cls(kind="registry", qualified_name=qualified_name, option_key=option_key, values=values) + + def to_dict(self) -> dict[str, object]: + return { + "kind": self.kind, + "qualifiedName": self.qualified_name, + "optionKey": self.option_key, + "values": dict(self.values), + } + + def values_dict(self) -> dict[str, str]: + return dict(self.values) + + def with_values(self, values: Mapping[str, str]) -> "RegistryInstallIntent": + return replace(self, values=tuple(sorted(values.items()))) + + +@dataclass(frozen=True) +class ManagedMcpRecord: + spec: McpServerSpec + install_intent: RegistryInstallIntent | None = None + + def to_dict(self) -> dict[str, object]: + payload = self.spec.to_dict() + if self.install_intent is not None: + payload["installIntent"] = self.install_intent.to_dict() + return payload + + @classmethod + def from_dict(cls, payload: Mapping[str, object]) -> "ManagedMcpRecord": + spec = McpServerSpec.from_dict(payload) + raw_intent = payload.get("installIntent") + install_intent = ( + RegistryInstallIntent.from_dict(raw_intent) + if isinstance(raw_intent, Mapping) + else None + ) + return cls(spec=spec, install_intent=install_intent) + + +def registry_record_from_detail( + detail: Mapping[str, object], + *, + config: Mapping[str, object] | None = None, + allow_missing_required: bool = False, +) -> ManagedMcpRecord: + option = _selected_option(detail, None) + values = resolved_config_values( + option.fields, + config or {}, + allow_missing_required=allow_missing_required, + ) + spec = resolve_registry_server_spec( + detail, + config=values, + allow_missing_required=allow_missing_required, + option_key=option.option_key, + ) + return ManagedMcpRecord( + spec=spec, + install_intent=RegistryInstallIntent.create( + qualified_name=spec.source.locator, + option_key=option.option_key, + values=values, + ), + ) + + +def install_intent_config_status( + detail: Mapping[str, object], + intent: RegistryInstallIntent | None, + *, + legacy_values: Mapping[str, str] | None = None, +) -> tuple[bool, tuple[str, ...]]: + option = _selected_option(detail, intent.option_key if intent else None) + fields = option.fields + if not fields: + return False, () + values = intent.values_dict() if intent is not None else dict(legacy_values or {}) + missing = tuple( + field.name + for field in fields + if field.required and field.default is None and not _has_value(values.get(field.name)) + ) + return True, missing + + +def resolve_enable_record( + detail: Mapping[str, object], + record: ManagedMcpRecord, + *, + config: Mapping[str, object] | None, + legacy_values: Mapping[str, str] | None = None, +) -> ManagedMcpRecord: + if record.spec.source.kind != "marketplace": + return record + intent = record.install_intent or _legacy_registry_intent(detail, record, legacy_values or {}) + option = _selected_option(detail, intent.option_key) + current = intent.values_dict() + if config is None: + missing = tuple( + field.name + for field in option.fields + if field.required and field.default is None and not _has_value(current.get(field.name)) + ) + if missing: + raise MutationError( + f"missing required install config: {', '.join(missing)}", + status=400, + ) + values = current + else: + merged: dict[str, object] = dict(current) + for key, value in config.items(): + if value is None or value == "": + continue + merged[key] = value + values = resolved_config_values(option.fields, merged) + + resolved = resolve_registry_server_spec(detail, config=values, option_key=option.option_key) + spec = replace( + resolved, + name=record.spec.name, + display_name=record.spec.display_name, + source=record.spec.source, + installed_at=record.spec.installed_at, + ) + return ManagedMcpRecord(spec=spec, install_intent=intent.with_values(values)) + + +def _legacy_registry_intent( + detail: Mapping[str, object], + record: ManagedMcpRecord, + values: Mapping[str, str], +) -> RegistryInstallIntent: + option = _selected_option(detail, None) + return RegistryInstallIntent.create( + qualified_name=record.spec.source.locator, + option_key=option.option_key, + values=values, + ) + + +def _selected_option( + detail: Mapping[str, object], + option_key: str | None, +) -> RegistryInstallOption: + option = registry_install_option_by_key(detail, option_key) if option_key else None + if option is None: + option = registry_install_option(detail) + if option is None: + raise MutationError("registry server has no supported install configuration", status=400) + return option + + +def _has_value(value: object) -> bool: + return isinstance(value, str) and value != "" + + +__all__ = [ + "ManagedMcpRecord", + "RegistryInstallIntent", + "install_intent_config_status", + "registry_record_from_detail", + "resolve_enable_record", +] diff --git a/skill_manager/application/mcp/install_resolver.py b/skill_manager/application/mcp/install_resolver.py index 21a9bf1..e0668a6 100644 --- a/skill_manager/application/mcp/install_resolver.py +++ b/skill_manager/application/mcp/install_resolver.py @@ -31,6 +31,7 @@ @dataclass(frozen=True) class RegistryInstallOption: + option_key: str transport: Literal["stdio", "http", "sse"] command: str | None = None args: tuple[str, ...] | None = None @@ -47,6 +48,7 @@ class _PackageInstallInput: registry_type: str identifier: str version: str + option_key: str fields: tuple[McpInstallConfigField, ...] env_bindings: tuple[EnvBinding, ...] argument_bindings: tuple[ArgumentBinding, ...] @@ -68,11 +70,22 @@ def registry_install_option(detail: Mapping[str, object]) -> RegistryInstallOpti return options[0] if options else None +def registry_install_option_by_key( + detail: Mapping[str, object], + option_key: str, +) -> RegistryInstallOption | None: + return next( + (option for option in registry_install_options(_server_payload(detail)) if option.option_key == option_key), + None, + ) + + def resolve_registry_server_spec( detail: Mapping[str, object], *, config: Mapping[str, object] | None = None, allow_missing_required: bool = False, + option_key: str | None = None, ) -> McpServerSpec: server = _server_payload(detail) qualified_name = _str(detail.get("qualifiedName")) or _str(server.get("name")) @@ -85,7 +98,9 @@ def resolve_registry_server_spec( f"registry server '{qualified_name}' has no supported install configuration", status=400, ) - option = options[0] + option = next((candidate for candidate in options if candidate.option_key == option_key), None) if option_key else None + if option is None: + option = options[0] common = { "name": registry_managed_name(qualified_name), "display_name": display_name, @@ -177,10 +192,13 @@ def _package_install_input(package: object, server: Mapping[str, object]) -> _Pa argument_bindings: list[ArgumentBinding] = [] fields.extend(argument_fields_and_bindings(package.get("runtimeArguments"), "runtimeArgument", argument_bindings)) fields.extend(argument_fields_and_bindings(package.get("packageArguments"), "packageArgument", argument_bindings)) + registry_type = _str(package.get("registryType")).lower() + version = _str(package.get("version")) or _str(server.get("version")) return _PackageInstallInput( - registry_type=_str(package.get("registryType")).lower(), + registry_type=registry_type, identifier=identifier, - version=_str(package.get("version")) or _str(server.get("version")), + version=version, + option_key=_package_option_key(registry_type, identifier, version), fields=dedupe_fields(fields), env_bindings=tuple(env_bindings), argument_bindings=tuple(argument_bindings), @@ -193,6 +211,7 @@ def _build_package_option(install_input: _PackageInstallInput) -> RegistryInstal return None command, args = command_builder(install_input.identifier, install_input.version) return RegistryInstallOption( + option_key=install_input.option_key, transport="stdio", command=command, args=args, @@ -223,6 +242,7 @@ def _remote_options(server: Mapping[str, object]) -> list[RegistryInstallOption] fields.extend(header_fields_and_bindings(remote, header_bindings)) options.append( RegistryInstallOption( + option_key=_remote_option_key(remote_type, url), transport=transport, url=url, fields=dedupe_fields(fields), @@ -264,6 +284,14 @@ def _versioned_oci_identifier(identifier: str, version: str) -> str: return f"{identifier}:{version}" +def _package_option_key(registry_type: str, identifier: str, version: str) -> str: + return f"package:{registry_type}:{identifier}:{version}" + + +def _remote_option_key(remote_type: str, url: str) -> str: + return f"remote:{remote_type}:{url}" + + def _npm_package_command(identifier: str, version: str) -> tuple[str, tuple[str, ...]]: return "npx", ("-y", _versioned_npm_identifier(identifier, version)) @@ -311,6 +339,7 @@ def _optional_str(value: object) -> str | None: "RegistryInstallOption", "registry_install_config", "registry_install_option", + "registry_install_option_by_key", "registry_install_options", "registry_managed_name", "resolve_registry_server_spec", diff --git a/skill_manager/application/mcp/install_state.py b/skill_manager/application/mcp/install_state.py index 79cde1b..fc891e4 100644 --- a/skill_manager/application/mcp/install_state.py +++ b/skill_manager/application/mcp/install_state.py @@ -1,16 +1,19 @@ from __future__ import annotations import re -from dataclasses import dataclass, replace +from dataclasses import dataclass from typing import Mapping -from skill_manager.errors import MutationError - from .install_config import ArgumentBinding +from .install_intent import ( + ManagedMcpRecord, + RegistryInstallIntent, + install_intent_config_status, + resolve_enable_record as resolve_enable_managed_record, +) from .install_resolver import ( RegistryInstallOption, registry_install_option, - resolve_registry_server_spec, ) from .store import McpServerSpec @@ -35,11 +38,15 @@ def to_dict(self) -> dict[str, object]: def install_config_status( detail: Mapping[str, object] | None, spec: McpServerSpec | None, + install_intent: RegistryInstallIntent | None = None, ) -> McpInstallConfigStatus: option = registry_install_option(detail or {}) fields = option.fields if option is not None else () if not fields: return McpInstallConfigStatus() + if install_intent is not None: + has_fields, missing = install_intent_config_status(detail or {}, install_intent) + return McpInstallConfigStatus(has_fields=has_fields, missing_required=missing) values = current_install_config_values(option, spec) if spec is not None else {} missing = tuple( field.name @@ -49,37 +56,21 @@ def install_config_status( return McpInstallConfigStatus(has_fields=True, missing_required=missing) -def resolve_enable_spec( +def resolve_enable_managed_spec( detail: Mapping[str, object], - spec: McpServerSpec, + record: ManagedMcpRecord, *, config: Mapping[str, object] | None, -) -> McpServerSpec: - option = registry_install_option(detail) - if option is None: - return spec - status = install_config_status(detail, spec) - if config is None: - if status.missing_required: - raise MutationError( - f"missing required install config: {', '.join(status.missing_required)}", - status=400, - ) - return spec - - merged_config: dict[str, object] = dict(current_install_config_values(option, spec)) - for key, value in config.items(): - if value is None or value == "": - continue - merged_config[key] = value - - resolved = resolve_registry_server_spec(detail, config=merged_config) - return replace( - resolved, - name=spec.name, - display_name=spec.display_name, - source=spec.source, - installed_at=spec.installed_at, +) -> ManagedMcpRecord: + legacy_values: Mapping[str, str] | None = None + if record.install_intent is None and record.spec.source.kind == "marketplace": + option = registry_install_option(detail) + legacy_values = current_install_config_values(option, record.spec) if option is not None else {} + return resolve_enable_managed_record( + detail, + record, + config=config, + legacy_values=legacy_values, ) @@ -270,5 +261,5 @@ def _has_value(value: object) -> bool: "McpInstallConfigStatus", "current_install_config_values", "install_config_status", - "resolve_enable_spec", + "resolve_enable_managed_spec", ] diff --git a/skill_manager/application/mcp/managed_state.py b/skill_manager/application/mcp/managed_state.py new file mode 100644 index 0000000..3622077 --- /dev/null +++ b/skill_manager/application/mcp/managed_state.py @@ -0,0 +1,204 @@ +from __future__ import annotations + +from typing import Callable, Mapping + +from .availability import McpAvailabilityResult, availability_cache_key +from .config_choice import config_choices_payload +from .contracts import McpBinding, McpHarnessScan, McpInventory, McpInventoryEntry +from .install_intent import ManagedMcpRecord +from .install_state import McpInstallConfigStatus, install_config_status +from .redaction import annotate_redacted_env, redacted_spec_dict +from .store import McpServerSpec + + +InstallDetailLookup = Callable[[str], Mapping[str, object] | None] + + +def inventory_payload( + inventory: McpInventory, + scans: tuple[McpHarnessScan, ...], + *, + records: Mapping[str, ManagedMcpRecord], + availability_cache: Mapping[tuple[str, str], McpAvailabilityResult], + install_detail_lookup: InstallDetailLookup | None, +) -> dict[str, object]: + visible_harnesses = {scan.harness for scan in scans} + return { + "columns": [ + { + "harness": scan.harness, + "label": scan.label, + "logoKey": scan.logo_key, + "installed": scan.installed, + "configPresent": scan.config_present, + "mcpWritable": scan.mcp_writable, + "mcpUnavailableReason": scan.mcp_unavailable_reason, + } + for scan in scans + ], + "entries": [ + entry_payload( + entry, + scans, + records=records, + availability=availability_cache.get(_availability_cache_key(entry)), + install_detail_lookup=install_detail_lookup, + ) + for entry in inventory.entries + if entry.kind == "managed" + or any(binding.harness in visible_harnesses for binding in entry.sightings) + ], + "issues": [ + {"name": issue.name, "reason": issue.reason} + for issue in inventory.issues + ], + } + + +def entry_payload( + entry: McpInventoryEntry, + scans: tuple[McpHarnessScan, ...], + *, + records: Mapping[str, ManagedMcpRecord], + availability: McpAvailabilityResult | None, + install_detail_lookup: InstallDetailLookup | None, +) -> dict[str, object]: + visible_harnesses = {scan.harness for scan in scans} + addressable_harnesses = _addressable_harnesses(scans) + spec_payload = redacted_spec_dict(entry.spec) if entry.spec is not None else None + enabled_status = _entry_enabled_status(entry, addressable_harnesses) + effective_availability = _entry_effective_availability(availability) + config_status = install_config_status_for_entry(entry, records, install_detail_lookup) + return { + "name": entry.name, + "displayName": entry.display_name, + "kind": entry.kind, + "spec": spec_payload, + "canEnable": entry.can_enable, + "enabledStatus": enabled_status, + "availabilityStatus": effective_availability.status, + "availabilityReason": effective_availability.reason, + "mcpStatus": mcp_status(availability, config_status), + "installConfigStatus": config_status.to_dict(), + "sightings": [ + _binding_to_dict(binding) + for binding in entry.sightings + if binding.harness in visible_harnesses + ], + } + + +def detail_extras_payload( + *, + name: str, + spec: McpServerSpec, + scans: tuple[McpHarnessScan, ...], +) -> dict[str, object]: + return { + "env": annotate_redacted_env(spec.env), + "configChoices": config_choices_payload(name, spec, scans), + } + + +def install_config_status_for_entry( + entry: McpInventoryEntry, + records: Mapping[str, ManagedMcpRecord], + install_detail_lookup: InstallDetailLookup | None, +) -> McpInstallConfigStatus: + spec = entry.spec + if spec is None or spec.source.kind != "marketplace" or install_detail_lookup is None: + return McpInstallConfigStatus() + detail = install_detail_lookup(spec.source.locator) + if detail is None: + return McpInstallConfigStatus() + record = records.get(spec.name) + return install_config_status( + detail, + spec, + record.install_intent if record is not None else None, + ) + + +def mcp_status( + availability: McpAvailabilityResult | None, + install_config_status: McpInstallConfigStatus, +) -> dict[str, object]: + if install_config_status.missing_required: + return { + "kind": "needs_config", + "reason": None, + } + if availability is None: + return { + "kind": "unchecked", + "reason": None, + } + if availability.status == "available": + return { + "kind": "available", + "reason": None, + } + if not availability.reason: + return { + "kind": "unchecked", + "reason": None, + } + return { + "kind": "connection_issue", + "reason": availability.reason, + } + + +def _binding_to_dict(binding: McpBinding) -> dict[str, object]: + payload: dict[str, object] = { + "harness": binding.harness, + "state": binding.state, + } + if binding.drift_detail: + payload["driftDetail"] = binding.drift_detail + return payload + + +def _is_scan_addressable(scan: McpHarnessScan) -> bool: + return scan.mcp_writable and (scan.installed or scan.config_present) + + +def _addressable_harnesses(scans: tuple[McpHarnessScan, ...]) -> set[str]: + return { + scan.harness + for scan in scans + if _is_scan_addressable(scan) + } + + +def _entry_enabled_status( + entry: McpInventoryEntry, + addressable_harnesses: set[str], +) -> str: + for binding in entry.sightings: + if binding.harness in addressable_harnesses and binding.state == "managed": + return "enabled" + return "disabled" + + +def _availability_cache_key(entry: McpInventoryEntry) -> tuple[str, str]: + if entry.spec is None: + return (entry.name, "") + return availability_cache_key(entry.name, entry.spec) + + +def _entry_effective_availability( + availability: McpAvailabilityResult | None, +) -> McpAvailabilityResult: + if availability is None: + return McpAvailabilityResult(status="unavailable", reason=None) + return availability + + +__all__ = [ + "detail_extras_payload", + "entry_payload", + "install_config_status_for_entry", + "inventory_payload", + "mcp_status", +] diff --git a/skill_manager/application/mcp/marketplace/catalog.py b/skill_manager/application/mcp/marketplace/catalog.py index 94f2317..c300713 100644 --- a/skill_manager/application/mcp/marketplace/catalog.py +++ b/skill_manager/application/mcp/marketplace/catalog.py @@ -12,10 +12,15 @@ from ..install_resolver import ( McpInstallConfig, RegistryInstallOption, - registry_install_options, registry_managed_name, ) from .client import McpRegistryClient +from .support_policy import ( + is_latest_active_registry_entry, + official_registry_meta, + registry_entry_server, + supported_registry_entry, +) Fetcher = Callable[[str], dict[str, object]] @@ -24,7 +29,6 @@ _REGISTRY_API_VERSION = "v0.1" _REGISTRY_EXTERNAL_BASE_URL = "https://registry.modelcontextprotocol.io" -_OFFICIAL_META_KEY = "io.modelcontextprotocol.registry/official" _DEFAULT_PAGE_SIZE = 20 _MAX_PAGE_SIZE = 60 _POPULAR_TTL_SECONDS = 3600 @@ -191,14 +195,10 @@ def _latest_supported_detail( if error.upstream_status == 404: return None raise - if not _is_latest_active(raw): - return None - detail_server = _entry_server(raw) - if detail_server is None: - return None - options = registry_install_options(detail_server) - if not options: + resolved = supported_registry_entry(raw) + if resolved is None: return None + _detail_server, options = resolved return raw, options def _search_snapshot( @@ -306,21 +306,15 @@ def _entries(raw: Mapping[str, object]) -> list[Mapping[str, object]]: def _entry_server(entry: Mapping[str, object]) -> Mapping[str, object] | None: - server = entry.get("server") - return server if isinstance(server, Mapping) else None + return registry_entry_server(entry) def _official_meta(entry: Mapping[str, object]) -> Mapping[str, object]: - meta = entry.get("_meta") - if not isinstance(meta, Mapping): - return {} - official = meta.get(_OFFICIAL_META_KEY) - return official if isinstance(official, Mapping) else {} + return official_registry_meta(entry) def _is_latest_active(entry: Mapping[str, object]) -> bool: - meta = _official_meta(entry) - return meta.get("status") == "active" and meta.get("isLatest") is True + return is_latest_active_registry_entry(entry) def _latest_active_entry(raw: Mapping[str, object]) -> Mapping[str, object] | None: @@ -342,12 +336,10 @@ def _supported_latest_entries( ) -> list[SupportedRegistryEntry]: supported: list[SupportedRegistryEntry] = [] for entry in _entries(raw): - server = _entry_server(entry) - if server is None or not _is_latest_active(entry): - continue - options = registry_install_options(server) - if not options: + resolved = supported_registry_entry(entry) + if resolved is None: continue + server, options = resolved supported.append((entry, server, options)) return supported diff --git a/skill_manager/application/mcp/marketplace/support_policy.py b/skill_manager/application/mcp/marketplace/support_policy.py new file mode 100644 index 0000000..f4372db --- /dev/null +++ b/skill_manager/application/mcp/marketplace/support_policy.py @@ -0,0 +1,48 @@ +from __future__ import annotations + +from typing import Mapping + +from ..install_resolver import RegistryInstallOption, registry_install_options + + +_OFFICIAL_META_KEY = "io.modelcontextprotocol.registry/official" + + +def is_latest_active_registry_entry(entry: Mapping[str, object]) -> bool: + meta = official_registry_meta(entry) + return meta.get("status") == "active" and meta.get("isLatest") is True + + +def official_registry_meta(entry: Mapping[str, object]) -> Mapping[str, object]: + meta = entry.get("_meta") + if not isinstance(meta, Mapping): + return {} + official = meta.get(_OFFICIAL_META_KEY) + return official if isinstance(official, Mapping) else {} + + +def registry_entry_server(entry: Mapping[str, object]) -> Mapping[str, object] | None: + server = entry.get("server") + return server if isinstance(server, Mapping) else None + + +def supported_registry_entry( + entry: Mapping[str, object], +) -> tuple[Mapping[str, object], tuple[RegistryInstallOption, ...]] | None: + if not is_latest_active_registry_entry(entry): + return None + server = registry_entry_server(entry) + if server is None: + return None + options = registry_install_options(server) + if not options: + return None + return server, options + + +__all__ = [ + "is_latest_active_registry_entry", + "official_registry_meta", + "registry_entry_server", + "supported_registry_entry", +] diff --git a/skill_manager/application/mcp/mutations.py b/skill_manager/application/mcp/mutations.py index 8408928..06f7017 100644 --- a/skill_manager/application/mcp/mutations.py +++ b/skill_manager/application/mcp/mutations.py @@ -10,9 +10,11 @@ McpAvailabilityProbe, availability_cache_key, ) +from .config_choice import observed_spec_from_scans from .enrichment import McpEnrichmentService -from .install_resolver import resolve_registry_server_spec -from .install_state import resolve_enable_spec +from .harness_application import McpHarnessApplication +from .install_intent import ManagedMcpRecord, registry_record_from_detail +from .install_state import resolve_enable_managed_spec from .marketplace.catalog import McpMarketplaceCatalog from .planner import McpAdoptionPlanner from .read_models import McpReadModelService @@ -45,6 +47,7 @@ def __init__( self.enrichment = enrichment self.availability_probe = availability_probe or McpAvailabilityProbe() self._availability_cache = availability_cache if availability_cache is not None else {} + self.harness_application = McpHarnessApplication(read_models) # Install / uninstall --------------------------------------------------- @@ -64,17 +67,18 @@ def install_from_marketplace( detail = self._marketplace_install_detail(qualified_name) if detail is None: raise MutationError(f"server not found in marketplace: {qualified_name}", status=404) - source_spec = resolve_registry_server_spec( + source_record = registry_record_from_detail( detail, allow_missing_required=True, ) - if self.store.get_managed(source_spec.name) is not None: + if self.store.get_managed(source_record.spec.name) is not None: raise MutationError( - f"a server named '{source_spec.name}' is already installed", + f"a server named '{source_record.spec.name}' is already installed", status=409, ) - stored = self.store.upsert_from_spec(source_spec) + stored_record = self.store.upsert_record(source_record) + stored = stored_record.spec self.read_models.invalidate() self._availability_cache[availability_cache_key(stored.name, stored)] = ( self.availability_probe.probe(stored) @@ -85,25 +89,11 @@ def uninstall_server(self, name: str) -> dict[str, object]: if self.store.get_managed(name) is None: raise MutationError(f"unknown server: {name}", status=404) bound_harnesses = self._harnesses_in_states(name, {"managed", "drifted"}) - succeeded: list[str] = [] - failures: list[dict[str, str]] = [] - for adapter in self.read_models.enabled_adapters(): - if adapter.harness not in bound_harnesses: - continue - try: - adapter.disable_server(name) - succeeded.append(adapter.harness) - except Exception as error: # noqa: BLE001 - failures.append({"harness": adapter.harness, "error": str(error)}) - if not failures: - self.store.remove(name) - if succeeded or not failures: - self.read_models.invalidate() - return { - "ok": not failures, - "succeeded": succeeded, - "failed": failures, - } + return self.harness_application.disable_many( + name, + bound_harnesses, + remove_after_full_success=lambda: self.store.remove(name), + ).to_dict() # Per-harness toggle ---------------------------------------------------- @@ -114,15 +104,18 @@ def enable_server( *, config: dict[str, object] | None = None, ) -> dict[str, bool]: - spec = self._require_server(name) + record = self._require_record(name) adapter = self.read_models.require_enabled_adapter(harness) if adapter.has_binding(name): return {"ok": True} - binding_spec = self._binding_spec_for_enable(spec, config=config) - adapter.enable_server(binding_spec) - if binding_spec != spec: - self.store.upsert_from_spec(binding_spec) - self.read_models.invalidate() + binding_record = self._record_for_enable(record, config=config) + result = self.harness_application.enable_one( + adapter, + binding_record.spec, + commit=(lambda: self.store.upsert_record(binding_record)) if binding_record != record else None, + ) + if result.failed: + raise MutationError(result.failed[0]["error"], status=400) return {"ok": True} def disable_server(self, name: str, harness: str) -> dict[str, bool]: @@ -142,59 +135,36 @@ def set_server_all_harnesses( ) -> dict[str, object]: if target not in ("enabled", "disabled"): raise MutationError("target must be 'enabled' or 'disabled'", status=400) - spec = self._require_server(name) - binding_spec = self._binding_spec_for_enable(spec, config=config) if target == "enabled" else spec + record = self._require_record(name) + binding_record = self._record_for_enable(record, config=config) if target == "enabled" else record bound_now = self._harnesses_in_states(name, {"managed", "drifted"}) - - succeeded: list[str] = [] - failures: list[dict[str, str]] = [] - flipped_any = False - - adapters = ( - self.read_models.enabled_writable_adapters() - if target == "enabled" - else self.read_models.enabled_addressable_adapters() - ) - for adapter in adapters: - if target == "enabled" and adapter.harness in bound_now: - continue - if target == "disabled" and adapter.harness not in bound_now: - continue - try: - if target == "enabled": - adapter.enable_server(binding_spec) - else: - adapter.disable_server(name) - except Exception as error: # noqa: BLE001 - failures.append({"harness": adapter.harness, "error": str(error)}) - continue - succeeded.append(adapter.harness) - flipped_any = True - - if flipped_any: - if target == "enabled" and binding_spec != spec: - self.store.upsert_from_spec(binding_spec) - self.read_models.invalidate() - - return { - "ok": not failures, - "succeeded": succeeded, - "failed": failures, - } - - def _binding_spec_for_enable( + if target == "enabled": + return self.harness_application.enable_many( + binding_record.spec, + self.read_models.enabled_harnesses(), + writable_only=True, + skip_harnesses=bound_now, + commit=(lambda: self.store.upsert_record(binding_record)) if binding_record != record else None, + ).to_dict() + return self.harness_application.disable_many( + name, + bound_now, + addressable_only=True, + ).to_dict() + + def _record_for_enable( self, - spec: McpServerSpec, + record: ManagedMcpRecord, *, config: dict[str, object] | None, - ) -> McpServerSpec: - if spec.source.kind != "marketplace": - return spec - detail = self._marketplace_install_detail(spec.source.locator) + ) -> ManagedMcpRecord: + if record.spec.source.kind != "marketplace": + return record + detail = self._marketplace_install_detail(record.spec.source.locator) if detail is None: - raise MutationError(f"server not found in marketplace: {spec.source.locator}", status=404) - return resolve_enable_spec(detail, spec, config=config) + raise MutationError(f"server not found in marketplace: {record.spec.source.locator}", status=404) + return resolve_enable_managed_spec(detail, record, config=config) # Reconciliation ------------------------------------------------------- @@ -213,9 +183,10 @@ def reconcile_server( if harnesses is not None else self._harnesses_in_states(name, {"managed", "drifted"}, addressable_only=True) ) - current = self._require_server(name) + current_record = self._require_record(name) + current = current_record.spec if source_kind == "managed": - source_spec = current + source_record = current_record elif source_kind == "harness": if not observed_harness: raise MutationError("observedHarness is required when sourceKind is 'harness'", status=400) @@ -226,22 +197,21 @@ def reconcile_server( display_name=current.display_name, source=current.source, ) - self.store.upsert_from_spec(source_spec) - self.read_models.invalidate() - source_spec = self._require_server(name) + source_record = ManagedMcpRecord(spec=source_spec) else: raise MutationError("sourceKind must be 'managed' or 'harness'", status=400) - stored = self.store.get_public_spec(name) or source_spec - binding_spec = self.store.get_binding_spec(name) or source_spec - succeeded, failures = self._write_spec_to_harnesses(binding_spec, target_harnesses) - if succeeded: - self.read_models.invalidate() + result = self.harness_application.enable_many( + source_record.spec, + target_harnesses, + commit=(lambda: self.store.upsert_record(source_record)) if source_record != current_record else None, + ) + stored = self.store.get_public_spec(name) or source_record.spec return { - "ok": not failures, + "ok": result.ok, "server": redacted_spec_dict(stored), - "succeeded": succeeded, - "failed": failures, + "succeeded": result.succeeded, + "failed": result.failed, } # Adoption ------------------------------------------------------------- @@ -292,23 +262,19 @@ def adopt( target_spec = self._apply_enrichment(target_spec) target_harnesses = set(harnesses) if harnesses else {s.harness for s in group.sightings} - stored = self.store.upsert_from_spec(target_spec) - stored_binding_spec = self.store.get_binding_spec(stored.name) - if stored_binding_spec is None: - raise MutationError(f"unknown server: {name}", status=404) - - succeeded, failures = self._write_spec_to_harnesses( - stored_binding_spec, + target_record = ManagedMcpRecord(spec=target_spec) + result = self.harness_application.enable_many( + target_record.spec, target_harnesses, + commit=lambda: self.store.upsert_record(target_record), ) - self.read_models.invalidate() - response_spec = self.store.get_public_spec(stored.name) or stored_binding_spec + response_spec = self.store.get_public_spec(target_spec.name) or target_spec return { - "ok": not failures, + "ok": result.ok, "server": redacted_spec_dict(response_spec), - "succeeded": succeeded, - "failed": failures, + "succeeded": result.succeeded, + "failed": result.failed, } # Internal helpers ----------------------------------------------------- @@ -347,38 +313,7 @@ def _harnesses_in_states( def _observed_spec(self, name: str, harness: str) -> McpServerSpec: snapshot = self.read_models.snapshot() - for scan in snapshot.harness_scans: - if scan.harness != harness: - continue - for entry in scan.entries: - if entry.name != name: - continue - if entry.parsed_spec is None: - raise MutationError( - entry.parse_issue or f"unable to parse '{name}' in {harness}", - status=409, - ) - return entry.parsed_spec - raise MutationError(f"server '{name}' was not observed in harness '{harness}'", status=404) - - def _write_spec_to_harnesses( - self, - spec: McpServerSpec, - harnesses: Iterable[str], - ) -> tuple[list[str], list[dict[str, str]]]: - targets = set(harnesses) - succeeded: list[str] = [] - failures: list[dict[str, str]] = [] - for adapter in self.read_models.enabled_adapters(): - if adapter.harness not in targets: - continue - try: - adapter.enable_server(spec) - except Exception as error: # noqa: BLE001 - failures.append({"harness": adapter.harness, "error": str(error)}) - continue - succeeded.append(adapter.harness) - return succeeded, failures + return observed_spec_from_scans(name, harness, snapshot.harness_scans) def _require_server(self, name: str) -> McpServerSpec: spec = self.store.get_binding_spec(name) @@ -386,6 +321,12 @@ def _require_server(self, name: str) -> McpServerSpec: raise MutationError(f"unknown server: {name}", status=404) return spec + def _require_record(self, name: str) -> ManagedMcpRecord: + record = self.store.get_record(name) + if record is None: + raise MutationError(f"unknown server: {name}", status=404) + return record + def _managed_for_marketplace(self, qualified_name: str) -> McpServerSpec | None: for server in self.store.list_managed(): if server.source.kind == "marketplace" and server.source.locator == qualified_name: diff --git a/skill_manager/application/mcp/query.py b/skill_manager/application/mcp/query.py index 16c700f..bc3dc9b 100644 --- a/skill_manager/application/mcp/query.py +++ b/skill_manager/application/mcp/query.py @@ -1,24 +1,21 @@ from __future__ import annotations -from typing import Literal - from skill_manager.errors import MutationError -from .contracts import McpBinding, McpHarnessScan, McpInventory, McpInventoryIssue +from .config_choice import recommended_observed_harness +from .contracts import McpHarnessScan, McpInventory, McpInventoryIssue from .availability import ( AvailabilityCache, McpAvailabilityProbe, - McpAvailabilityResult, availability_cache_key, ) from .enrichment import McpEnrichmentService -from .install_state import McpInstallConfigStatus, install_config_status from .inventory import build_inventory +from .managed_state import detail_extras_payload, entry_payload, inventory_payload from .marketplace.catalog import McpMarketplaceCatalog from .planner import McpAdoptionPlanner from .read_models import McpReadModelService from .redaction import annotate_redacted_env, redact_payload, redacted_spec_dict -from .store import McpServerSpec class McpQueryService: @@ -44,12 +41,12 @@ def __init__( def list_servers(self) -> dict[str, object]: snapshot = self.read_models.snapshot() inventory = self._inventory(snapshot.harness_scans) - install_config_statuses = self._install_config_statuses(inventory) - return _inventory_to_payload( + return inventory_payload( inventory, self.read_models.visible_scans(snapshot), - self._availability_cache, - install_config_statuses, + records=self._records_by_name(), + availability_cache=self._availability_cache, + install_detail_lookup=self._marketplace_install_detail, ) def get_server(self, name: str) -> dict[str, object]: @@ -58,19 +55,15 @@ def get_server(self, name: str) -> dict[str, object]: visible_scans = self.read_models.visible_scans(snapshot) for entry in inventory.entries: if entry.name == name: - payload = _entry_to_payload( + payload = entry_payload( entry, visible_scans, - self._availability_cache.get(_availability_cache_key(entry)), - self._install_config_status_for_spec(entry.spec), + records=self._records_by_name(), + availability=self._availability_cache.get(_availability_cache_key(entry)), + install_detail_lookup=self._marketplace_install_detail, ) if entry.spec is not None: - payload["env"] = annotate_redacted_env(entry.spec.env) - payload["configChoices"] = _config_choices_payload( - name, - entry.spec, - visible_scans, - ) + payload.update(detail_extras_payload(name=name, spec=entry.spec, scans=visible_scans)) link = self.enrichment.lookup(name) if self.enrichment else None if link is not None: payload["marketplaceLink"] = link.to_dict() @@ -147,6 +140,7 @@ def list_unmanaged_by_server(self) -> dict[str, object]: ) if not sightings: continue + recommended_harness = recommended_observed_harness(sightings) sightings_payload = [ { "harness": s.harness, @@ -156,6 +150,7 @@ def list_unmanaged_by_server(self) -> dict[str, object]: "payloadPreview": redact_payload(s.payload), "spec": redacted_spec_dict(s.spec), "env": annotate_redacted_env(s.spec.env), + "recommended": s.harness == recommended_harness, } for s in sightings ] @@ -190,27 +185,12 @@ def _inventory(self, scans: tuple[McpHarnessScan, ...]) -> McpInventory: issues=issues, ) - def _install_config_statuses( - self, - inventory: McpInventory, - ) -> dict[str, McpInstallConfigStatus]: + def _records_by_name(self): return { - entry.name: self._install_config_status_for_spec(entry.spec) - for entry in inventory.entries - if entry.spec is not None + record.spec.name: record + for record in self.read_models.store.list_records() } - def _install_config_status_for_spec( - self, - spec: McpServerSpec | None, - ) -> McpInstallConfigStatus: - if spec is None or spec.source.kind != "marketplace" or self.marketplace is None: - return McpInstallConfigStatus() - detail = self._marketplace_install_detail(spec.source.locator) - if detail is None: - return McpInstallConfigStatus() - return install_config_status(detail, spec) - def _marketplace_install_detail(self, qualified_name: str): if self.marketplace is None: return None @@ -226,193 +206,10 @@ def _marketplace_install_detail(self, qualified_name: str): return None -def _binding_to_dict(binding: McpBinding) -> dict[str, object]: - payload: dict[str, object] = { - "harness": binding.harness, - "state": binding.state, - } - if binding.drift_detail: - payload["driftDetail"] = binding.drift_detail - return payload - - -def _is_scan_addressable(scan: McpHarnessScan) -> bool: - return scan.mcp_writable and (scan.installed or scan.config_present) - - -def _addressable_harnesses(scans: tuple[McpHarnessScan, ...]) -> set[str]: - return { - scan.harness - for scan in scans - if _is_scan_addressable(scan) - } - - -def _entry_enabled_status( - entry, - addressable_harnesses: set[str], -) -> Literal["enabled", "disabled"]: - for binding in entry.sightings: - if binding.harness in addressable_harnesses and binding.state == "managed": - return "enabled" - return "disabled" - - -def _entry_mcp_status( - entry, - availability: McpAvailabilityResult | None, - install_config_status: McpInstallConfigStatus, -) -> dict[str, object]: - if install_config_status.missing_required: - return { - "kind": "needs_config", - "reason": None, - } - if availability is None: - return { - "kind": "unchecked", - "reason": None, - } - if availability.status == "available": - return { - "kind": "available", - "reason": None, - } - if not availability.reason: - return { - "kind": "unchecked", - "reason": None, - } - return { - "kind": "connection_issue", - "reason": availability.reason, - } - - -def _entry_to_payload( - entry, - scans: tuple[McpHarnessScan, ...], - availability: McpAvailabilityResult | None = None, - config_status: McpInstallConfigStatus | None = None, -) -> dict[str, object]: - visible_harnesses = {scan.harness for scan in scans} - addressable_harnesses = _addressable_harnesses(scans) - spec_payload = redacted_spec_dict(entry.spec) if entry.spec is not None else None - enabled_status = _entry_enabled_status(entry, addressable_harnesses) - effective_availability = _entry_effective_availability(availability) - effective_config_status = config_status or McpInstallConfigStatus() - return { - "name": entry.name, - "displayName": entry.display_name, - "kind": entry.kind, - "spec": spec_payload, - "canEnable": entry.can_enable, - "enabledStatus": enabled_status, - "availabilityStatus": effective_availability.status, - "availabilityReason": effective_availability.reason, - "mcpStatus": _entry_mcp_status( - entry, - availability, - effective_config_status, - ), - "installConfigStatus": effective_config_status.to_dict(), - "sightings": [ - _binding_to_dict(binding) - for binding in entry.sightings - if binding.harness in visible_harnesses - ], - } - - def _availability_cache_key(entry) -> tuple[str, str]: if entry.spec is None: return (entry.name, "") return availability_cache_key(entry.name, entry.spec) -def _entry_effective_availability( - availability: McpAvailabilityResult | None, -) -> McpAvailabilityResult: - if availability is None: - return McpAvailabilityResult(status="unavailable", reason=None) - return availability - - -def _config_choices_payload( - name: str, - managed_spec, - scans: tuple[McpHarnessScan, ...], -) -> list[dict[str, object]]: - choices: list[dict[str, object]] = [ - { - "sourceKind": "managed", - "observedHarness": None, - "label": "Managed config", - "logoKey": None, - "configPath": None, - "payloadPreview": redacted_spec_dict(managed_spec), - "spec": redacted_spec_dict(managed_spec), - "env": annotate_redacted_env(managed_spec.env), - } - ] - for scan in scans: - for observed in scan.entries: - if observed.name != name or observed.state != "drifted": - continue - if observed.parsed_spec is None: - continue - choices.append( - { - "sourceKind": "harness", - "observedHarness": scan.harness, - "label": f"{scan.label} config", - "logoKey": scan.logo_key, - "configPath": str(scan.config_path) if scan.config_present else None, - "payloadPreview": redact_payload(dict(observed.raw_payload or {})), - "spec": redacted_spec_dict(observed.parsed_spec), - "env": annotate_redacted_env(observed.parsed_spec.env), - } - ) - return choices - - -def _inventory_to_payload( - inventory: McpInventory, - scans: tuple[McpHarnessScan, ...], - availability_cache: dict[tuple[str, str], McpAvailabilityResult] | None = None, - install_config_statuses: dict[str, McpInstallConfigStatus] | None = None, -) -> dict[str, object]: - visible_harnesses = {scan.harness for scan in scans} - statuses = install_config_statuses or {} - return { - "columns": [ - { - "harness": scan.harness, - "label": scan.label, - "logoKey": scan.logo_key, - "installed": scan.installed, - "configPresent": scan.config_present, - "mcpWritable": scan.mcp_writable, - "mcpUnavailableReason": scan.mcp_unavailable_reason, - } - for scan in scans - ], - "entries": [ - _entry_to_payload( - entry, - scans, - (availability_cache or {}).get(_availability_cache_key(entry)), - statuses.get(entry.name), - ) - for entry in inventory.entries - if entry.kind == "managed" - or any(binding.harness in visible_harnesses for binding in entry.sightings) - ], - "issues": [ - {"name": issue.name, "reason": issue.reason} - for issue in inventory.issues - ], - } - - __all__ = ["McpQueryService"] diff --git a/skill_manager/application/mcp/store.py b/skill_manager/application/mcp/store.py index 410c333..e27bfa9 100644 --- a/skill_manager/application/mcp/store.py +++ b/skill_manager/application/mcp/store.py @@ -5,14 +5,18 @@ from dataclasses import dataclass, field, replace from datetime import datetime, timezone from pathlib import Path -from typing import Literal, Mapping +from typing import TYPE_CHECKING, Literal, Mapping from skill_manager.atomic_files import atomic_write_text, file_lock McpTransport = Literal["stdio", "http", "sse"] McpSourceKind = Literal["marketplace", "adopted", "manual"] -CURRENT_MCP_MANIFEST_VERSION = 5 +if TYPE_CHECKING: + from .install_intent import ManagedMcpRecord + + +CURRENT_MCP_MANIFEST_VERSION = 6 @dataclass(frozen=True) @@ -120,7 +124,7 @@ def from_dict(cls, payload: Mapping[str, object]) -> "McpServerSpec": @dataclass(frozen=True) class McpManagedManifest: - entries: tuple[McpServerSpec, ...] = field(default_factory=tuple) + entries: tuple["ManagedMcpRecord", ...] = field(default_factory=tuple) def to_dict(self) -> dict[str, object]: return { @@ -182,6 +186,9 @@ def _lock_path(self) -> Path: return self.manifest_path.with_suffix(".lock") def list_managed(self) -> tuple[McpServerSpec, ...]: + return tuple(record.spec for record in self.list_records()) + + def list_records(self) -> tuple["ManagedMcpRecord", ...]: return self._load_manifest_result().manifest.entries def list_binding_specs(self) -> tuple[McpServerSpec, ...]: @@ -191,9 +198,13 @@ def list_public_specs(self) -> tuple[McpServerSpec, ...]: return self.list_managed() def get_managed(self, name: str) -> McpServerSpec | None: - for entry in self.list_managed(): - if entry.name == name: - return entry + record = self.get_record(name) + return record.spec if record is not None else None + + def get_record(self, name: str) -> "ManagedMcpRecord | None": + for record in self.list_records(): + if record.spec.name == name: + return record return None def get_binding_spec(self, name: str) -> McpServerSpec | None: @@ -205,14 +216,23 @@ def get_public_spec(self, name: str) -> McpServerSpec | None: def upsert_from_spec(self, spec: McpServerSpec) -> McpServerSpec: return self.upsert_managed(spec) + def upsert_record(self, record: "ManagedMcpRecord") -> "ManagedMcpRecord": + return self._upsert_record(record) + def upsert_managed(self, server: McpServerSpec) -> McpServerSpec: + existing = self.get_record(server.name) + record = self._record_from_spec(server, install_intent=existing.install_intent if existing else None) + return self._upsert_record(record).spec + + def _upsert_record(self, record: "ManagedMcpRecord") -> "ManagedMcpRecord": with file_lock(self._lock_path): manifest = self._load_manifest_result().manifest - stamped = prepare_managed_spec(server) + stamped_spec = prepare_managed_spec(record.spec) + stamped = self._record_from_spec(stamped_spec, install_intent=record.install_intent) new_entries = tuple( - stamped if entry.name == stamped.name else entry for entry in manifest.entries + stamped if entry.spec.name == stamped.spec.name else entry for entry in manifest.entries ) - if not any(entry.name == stamped.name for entry in manifest.entries): + if not any(entry.spec.name == stamped.spec.name for entry in manifest.entries): new_entries = manifest.entries + (stamped,) write_mcp_manifest(self.manifest_path, McpManagedManifest(entries=new_entries)) return stamped @@ -220,7 +240,7 @@ def upsert_managed(self, server: McpServerSpec) -> McpServerSpec: def remove(self, name: str) -> bool: with file_lock(self._lock_path): manifest = self._load_manifest_result().manifest - new_entries = tuple(entry for entry in manifest.entries if entry.name != name) + new_entries = tuple(entry for entry in manifest.entries if entry.spec.name != name) if len(new_entries) == len(manifest.entries): return False write_mcp_manifest(self.manifest_path, McpManagedManifest(entries=new_entries)) @@ -239,7 +259,7 @@ def _load_manifest_result(self) -> _ManifestLoadResult: McpManagedManifest(), issues=(McpManifestIssue(name="", reason="'servers' must be a list"),), ) - entries: list[McpServerSpec] = [] + records = [] issues: list[McpManifestIssue] = [] for item in raw_entries: if not isinstance(item, dict): @@ -247,15 +267,32 @@ def _load_manifest_result(self) -> _ManifestLoadResult: continue name = str(item.get("name", "")) try: - entries.append(McpServerSpec.from_dict(item)) + record = self._record_from_dict(item) + records.append(record) except (KeyError, TypeError, ValueError) as error: issues.append(McpManifestIssue(name=name, reason=str(error) or error.__class__.__name__)) continue return _ManifestLoadResult( - McpManagedManifest(entries=tuple(entries)), + McpManagedManifest(entries=tuple(records)), issues=tuple(issues), ) + @staticmethod + def _record_from_dict(payload: Mapping[str, object]) -> "ManagedMcpRecord": + from .install_intent import ManagedMcpRecord + + return ManagedMcpRecord.from_dict(payload) + + @staticmethod + def _record_from_spec( + spec: McpServerSpec, + *, + install_intent, + ) -> "ManagedMcpRecord": + from .install_intent import ManagedMcpRecord + + return ManagedMcpRecord(spec=spec, install_intent=install_intent) + __all__ = [ "CURRENT_MCP_MANIFEST_VERSION", diff --git a/tests/integration/test_mcp_routes.py b/tests/integration/test_mcp_routes.py index 14ee495..7410816 100644 --- a/tests/integration/test_mcp_routes.py +++ b/tests/integration/test_mcp_routes.py @@ -488,7 +488,7 @@ def test_marketplace_schema_metadata_does_not_change_observed_install(self) -> N self.assertEqual(install["server"]["url"], "https://mcp.exa.ai") self.assertFalse((harness.spec.home / ".cursor" / "mcp.json").exists()) - def test_install_defers_required_registry_environment_config(self) -> None: + def test_install_stores_required_registry_environment_config_without_harness_write(self) -> None: registry_server = { "name": "ai.cueapi/mcp", "title": "CueAPI", @@ -516,6 +516,13 @@ def test_install_defers_required_registry_environment_config(self) -> None: self.assertTrue(install["ok"]) self.assertEqual(probe.probed, ["ai-cueapi-mcp"]) self.assertIsNotNone(harness.container.mcp_store.get_managed("ai-cueapi-mcp")) + record = harness.container.mcp_store.get_record("ai-cueapi-mcp") + self.assertIsNotNone(record) + assert record is not None + self.assertIsNotNone(record.install_intent) + assert record.install_intent is not None + self.assertEqual(record.install_intent.option_key, "package:npm:@cueapi/mcp:0.1.3") + self.assertEqual(record.install_intent.values_dict(), {}) self.assertFalse((harness.spec.home / ".cursor" / "mcp.json").exists()) detail = harness.get_json("/api/mcp/servers/ai-cueapi-mcp") self.assertEqual(detail["installConfigStatus"]["hasFields"], True) @@ -567,6 +574,18 @@ def test_enable_writes_registry_environment_config_to_target_harness(self) -> No detail = harness.get_json("/api/mcp/servers/ai-cueapi-mcp") self.assertEqual(detail["installConfigStatus"]["missingRequired"], []) self.assertEqual(detail["installConfigStatus"]["configured"], True) + record = harness.container.mcp_store.get_record("ai-cueapi-mcp") + self.assertIsNotNone(record) + assert record is not None + self.assertIsNotNone(record.install_intent) + assert record.install_intent is not None + self.assertEqual( + record.install_intent.values_dict(), + { + "CUEAPI_API_KEY": "cue-key", + "CUEAPI_BASE_URL": "https://api.cueapi.ai", + }, + ) harness.post_json( "/api/mcp/servers/ai-cueapi-mcp/enable", @@ -699,6 +718,12 @@ def fail_enable(_spec: McpServerSpec) -> None: self.assertIsNotNone(managed) assert managed is not None self.assertNotIn("CUEAPI_API_KEY", managed.env_dict()) + record = harness.container.mcp_store.get_record("ai-cueapi-mcp") + self.assertIsNotNone(record) + assert record is not None + self.assertIsNotNone(record.install_intent) + assert record.install_intent is not None + self.assertNotIn("CUEAPI_API_KEY", record.install_intent.values_dict()) def test_enable_all_with_config_updates_manifest_after_partial_success(self) -> None: registry_server = { @@ -740,6 +765,12 @@ def fail_enable(_spec: McpServerSpec) -> None: self.assertIsNotNone(managed) assert managed is not None self.assertEqual(managed.env_dict()["CUEAPI_API_KEY"], "cue-key") + record = harness.container.mcp_store.get_record("ai-cueapi-mcp") + self.assertIsNotNone(record) + assert record is not None + self.assertIsNotNone(record.install_intent) + assert record.install_intent is not None + self.assertEqual(record.install_intent.values_dict()["CUEAPI_API_KEY"], "cue-key") def test_enable_all_with_config_leaves_manifest_when_every_write_fails(self) -> None: registry_server = { @@ -780,6 +811,12 @@ def fail_enable(_spec: McpServerSpec) -> None: self.assertIsNotNone(managed) assert managed is not None self.assertNotIn("CUEAPI_API_KEY", managed.env_dict()) + record = harness.container.mcp_store.get_record("ai-cueapi-mcp") + self.assertIsNotNone(record) + assert record is not None + self.assertIsNotNone(record.install_intent) + assert record.install_intent is not None + self.assertNotIn("CUEAPI_API_KEY", record.install_intent.values_dict()) def test_optional_config_is_preserved_when_enabling_another_harness(self) -> None: registry_server = { @@ -826,6 +863,12 @@ def test_optional_config_is_preserved_when_enabling_another_harness(self) -> Non self.assertIsNotNone(managed_after) assert managed_after is not None self.assertEqual(managed_after.env_dict()["OPTIONAL_TOKEN"], "opt-token") + record_after = harness.container.mcp_store.get_record("ai-optional-mcp") + self.assertIsNotNone(record_after) + assert record_after is not None + self.assertIsNotNone(record_after.install_intent) + assert record_after.install_intent is not None + self.assertEqual(record_after.install_intent.values_dict()["OPTIONAL_TOKEN"], "opt-token") claude_cfg = json.loads((harness.spec.home / ".claude.json").read_text()) self.assertEqual( claude_cfg["mcpServers"]["ai-optional-mcp"]["env"], diff --git a/tests/unit/test_mcp_config_choice.py b/tests/unit/test_mcp_config_choice.py new file mode 100644 index 0000000..142f6d9 --- /dev/null +++ b/tests/unit/test_mcp_config_choice.py @@ -0,0 +1,95 @@ +from __future__ import annotations + +import unittest +from pathlib import Path + +from skill_manager.application.mcp.config_choice import config_choices_payload, recommended_observed_harness +from skill_manager.application.mcp.contracts import McpHarnessScan, McpObservedEntry +from skill_manager.application.mcp.store import McpServerSpec, McpSource + + +def _stdio_spec(name: str, harness: str, env: tuple[tuple[str, str], ...] | None = None) -> McpServerSpec: + return McpServerSpec( + name=name, + display_name=name, + source=McpSource.adopted(harness, name), + transport="stdio", + command="uvx", + args=(f"{name}-mcp",), + env=env, + ) + + +def _http_spec(name: str, harness: str, url: str) -> McpServerSpec: + return McpServerSpec( + name=name, + display_name=name, + source=McpSource.adopted(harness, name), + transport="http", + url=url, + ) + + +class ConfigChoiceTests(unittest.TestCase): + def test_config_choices_have_stable_ids_and_one_recommendation(self) -> None: + managed = _http_spec("exa", "managed", "https://managed.example") + scans = ( + McpHarnessScan( + harness="cursor", + label="Cursor", + logo_key="cursor", + installed=True, + config_present=True, + config_path=Path("/tmp/cursor.json"), + entries=( + McpObservedEntry( + name="exa", + state="drifted", + raw_payload={"command": "uvx", "env": {"EXA_API_KEY": "${EXA_API_KEY}"}}, + parsed_spec=_stdio_spec("exa", "cursor", env=(("EXA_API_KEY", "${EXA_API_KEY}"),)), + ), + ), + ), + McpHarnessScan( + harness="claude", + label="Claude", + logo_key="claude", + installed=True, + config_present=True, + config_path=Path("/tmp/claude.json"), + entries=( + McpObservedEntry( + name="exa", + state="drifted", + raw_payload={"url": "https://remote.example"}, + parsed_spec=_http_spec("exa", "claude", "https://remote.example"), + ), + ), + ), + ) + + choices = config_choices_payload("exa", managed, scans) + + self.assertEqual([choice["id"] for choice in choices], ["managed", "harness:cursor", "harness:claude"]) + recommended = [choice for choice in choices if choice["recommended"]] + self.assertEqual(len(recommended), 1) + self.assertEqual(recommended[0]["id"], "harness:cursor") + + def test_recommended_observed_harness_prefers_stdio_env_reference(self) -> None: + class Sighting: + def __init__(self, harness: str, spec: McpServerSpec) -> None: + self.harness = harness + self.spec = spec + + selected = recommended_observed_harness( + [ + Sighting("claude", _http_spec("exa", "claude", "https://remote.example")), + Sighting("cursor", _stdio_spec("exa", "cursor", env=(("EXA_API_KEY", "${EXA_API_KEY}"),))), + ] + ) + + self.assertEqual(selected, "cursor") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_mcp_harness_application.py b/tests/unit/test_mcp_harness_application.py new file mode 100644 index 0000000..9b8d16e --- /dev/null +++ b/tests/unit/test_mcp_harness_application.py @@ -0,0 +1,103 @@ +from __future__ import annotations + +import unittest +from pathlib import Path + +from skill_manager.application.mcp.harness_application import McpHarnessApplication +from skill_manager.application.mcp.store import McpServerSpec, McpSource + + +def _spec() -> McpServerSpec: + return McpServerSpec( + name="exa", + display_name="Exa", + source=McpSource.marketplace("exa"), + transport="http", + url="https://mcp.exa.ai", + ) + + +class _Adapter: + label = "Harness" + logo_key = None + config_path = Path("/tmp/config.json") + + def __init__(self, harness: str, *, fail_enable: bool = False) -> None: + self.harness = harness + self.fail_enable = fail_enable + self.enabled: list[str] = [] + + def enable_server(self, spec: McpServerSpec) -> None: + if self.fail_enable: + raise RuntimeError(f"{self.harness} write failed") + self.enabled.append(spec.name) + + def disable_server(self, _name: str) -> None: + return None + + +class _ReadModels: + def __init__(self, adapters: tuple[_Adapter, ...]) -> None: + self.adapters = adapters + self.invalidated = 0 + + def enabled_adapters(self) -> tuple[_Adapter, ...]: + return self.adapters + + def enabled_writable_adapters(self) -> tuple[_Adapter, ...]: + return self.adapters + + def enabled_addressable_adapters(self) -> tuple[_Adapter, ...]: + return self.adapters + + def invalidate(self) -> None: + self.invalidated += 1 + + +class HarnessApplicationTests(unittest.TestCase): + def test_enable_many_commits_once_after_partial_success(self) -> None: + read_models = _ReadModels( + ( + _Adapter("cursor", fail_enable=True), + _Adapter("claude"), + ) + ) + app = McpHarnessApplication(read_models) # type: ignore[arg-type] + commits = 0 + + def commit() -> None: + nonlocal commits + commits += 1 + + result = app.enable_many(_spec(), {"cursor", "claude"}, commit=commit) + + self.assertFalse(result.ok) + self.assertEqual(result.succeeded, ["claude"]) + self.assertEqual(result.failed[0]["harness"], "cursor") + self.assertEqual(commits, 1) + self.assertEqual(read_models.invalidated, 1) + + def test_enable_many_skips_commit_when_all_writes_fail(self) -> None: + read_models = _ReadModels( + ( + _Adapter("cursor", fail_enable=True), + _Adapter("claude", fail_enable=True), + ) + ) + app = McpHarnessApplication(read_models) # type: ignore[arg-type] + commits = 0 + + def commit() -> None: + nonlocal commits + commits += 1 + + result = app.enable_many(_spec(), {"cursor", "claude"}, commit=commit) + + self.assertFalse(result.ok) + self.assertEqual(result.succeeded, []) + self.assertEqual(commits, 0) + self.assertEqual(read_models.invalidated, 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_mcp_historical_terms.py b/tests/unit/test_mcp_historical_terms.py new file mode 100644 index 0000000..406057f --- /dev/null +++ b/tests/unit/test_mcp_historical_terms.py @@ -0,0 +1,45 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + + +def test_mcp_historical_terms_do_not_reappear() -> None: + repo_root = Path(__file__).resolve().parents[2] + result = subprocess.run( + ["git", "ls-files", "-z"], + cwd=repo_root, + check=True, + capture_output=True, + ) + tracked_paths = [Path(path) for path in result.stdout.decode().split("\0") if path] + forbidden = [ + "smith" + "ery", + "source" + "harness", + "source" + "_" + "harness", + "source" + " " + "harness", + "deferred" + " install", + "deferred" + " mcp", + "registry" + " record", + "source" + " installer", + ] + + matches: list[str] = [] + for relative_path in tracked_paths: + path = repo_root / relative_path + try: + raw = path.read_bytes() + except FileNotFoundError: + continue + if b"\0" in raw: + continue + try: + text = raw.decode("utf-8") + except UnicodeDecodeError: + continue + for line_number, line in enumerate(text.splitlines(), start=1): + normalized = line.lower() + if any(term in normalized for term in forbidden): + matches.append(f"{relative_path}:{line_number}: {line.strip()}") + + assert matches == [] diff --git a/tests/unit/test_mcp_install_intent.py b/tests/unit/test_mcp_install_intent.py new file mode 100644 index 0000000..876bc71 --- /dev/null +++ b/tests/unit/test_mcp_install_intent.py @@ -0,0 +1,103 @@ +from __future__ import annotations + +import unittest + +from skill_manager.application.mcp.install_intent import ManagedMcpRecord, RegistryInstallIntent +from skill_manager.application.mcp.install_state import install_config_status, resolve_enable_managed_spec +from skill_manager.application.mcp.store import McpServerSpec, McpSource + + +def _detail() -> dict[str, object]: + return { + "qualifiedName": "ai.cueapi/mcp", + "displayName": "CueAPI", + "registryServer": { + "name": "ai.cueapi/mcp", + "title": "CueAPI", + "version": "0.1.3", + "packages": [ + { + "registryType": "npm", + "identifier": "@cueapi/mcp", + "version": "0.1.3", + "transport": {"type": "stdio"}, + "environmentVariables": [ + {"name": "CUEAPI_API_KEY", "isRequired": True, "isSecret": True}, + {"name": "CUEAPI_BASE_URL", "default": "https://api.cueapi.ai"}, + {"name": "OPTIONAL_TOKEN", "isSecret": True}, + ], + } + ], + }, + } + + +def _spec(env: tuple[tuple[str, str], ...] | None = None) -> McpServerSpec: + return McpServerSpec( + name="ai-cueapi-mcp", + display_name="CueAPI", + source=McpSource.marketplace("ai.cueapi/mcp"), + transport="stdio", + command="npx", + args=("-y", "@cueapi/mcp@0.1.3"), + env=env, + ) + + +class RegistryInstallIntentTests(unittest.TestCase): + def test_config_status_uses_persisted_intent_values(self) -> None: + intent = RegistryInstallIntent.create( + qualified_name="ai.cueapi/mcp", + option_key="package:npm:@cueapi/mcp:0.1.3", + values={"CUEAPI_API_KEY": "cue-key"}, + ) + + status = install_config_status(_detail(), _spec(env=None), intent) + + self.assertTrue(status.has_fields) + self.assertEqual(status.missing_required, ()) + self.assertTrue(status.configured) + + def test_enable_merge_keeps_existing_optional_values(self) -> None: + record = ManagedMcpRecord( + spec=_spec(env=None), + install_intent=RegistryInstallIntent.create( + qualified_name="ai.cueapi/mcp", + option_key="package:npm:@cueapi/mcp:0.1.3", + values={ + "CUEAPI_API_KEY": "cue-key", + "OPTIONAL_TOKEN": "saved-token", + }, + ), + ) + + resolved = resolve_enable_managed_spec(_detail(), record, config={}) + + self.assertEqual( + resolved.spec.env_dict(), + { + "CUEAPI_API_KEY": "cue-key", + "CUEAPI_BASE_URL": "https://api.cueapi.ai", + "OPTIONAL_TOKEN": "saved-token", + }, + ) + self.assertIsNotNone(resolved.install_intent) + assert resolved.install_intent is not None + self.assertEqual(resolved.install_intent.values_dict()["OPTIONAL_TOKEN"], "saved-token") + + def test_legacy_spec_only_record_gets_intent_on_enable_resolution(self) -> None: + record = ManagedMcpRecord( + spec=_spec(env=(("CUEAPI_API_KEY", "legacy-key"),)), + install_intent=None, + ) + + resolved = resolve_enable_managed_spec(_detail(), record, config=None) + + self.assertIsNotNone(resolved.install_intent) + assert resolved.install_intent is not None + self.assertEqual(resolved.install_intent.option_key, "package:npm:@cueapi/mcp:0.1.3") + self.assertEqual(resolved.install_intent.values_dict()["CUEAPI_API_KEY"], "legacy-key") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_mcp_install_resolver.py b/tests/unit/test_mcp_install_resolver.py index 9ef2cbd..56fea8d 100644 --- a/tests/unit/test_mcp_install_resolver.py +++ b/tests/unit/test_mcp_install_resolver.py @@ -4,6 +4,8 @@ from skill_manager.application.mcp.install_resolver import ( registry_install_config, + registry_install_option_by_key, + registry_install_options, registry_managed_name, resolve_registry_server_spec, ) @@ -43,6 +45,77 @@ def test_uses_full_registry_name_to_avoid_mcp_collisions(self) -> None: class RegistryInstallResolverTests(unittest.TestCase): + def test_install_option_keys_are_stable_for_supported_packages_and_remotes(self) -> None: + options = registry_install_options( + { + "name": "ai.example/mcp", + "version": "1.0.0", + "packages": [ + { + "registryType": "npm", + "identifier": "@example/npm-mcp", + "version": "1.0.0", + "transport": {"type": "stdio"}, + }, + { + "registryType": "pypi", + "identifier": "example-mcp", + "version": "2.0.0", + "transport": {"type": "stdio"}, + }, + { + "registryType": "oci", + "identifier": "ghcr.io/example/mcp", + "version": "3.0.0", + "transport": {"type": "stdio"}, + }, + ], + "remotes": [ + {"type": "streamable-http", "url": "https://api.example.com/mcp"}, + {"type": "sse", "url": "https://api.example.com/sse"}, + ], + } + ) + + self.assertEqual( + [option.option_key for option in options], + [ + "package:npm:@example/npm-mcp:1.0.0", + "package:pypi:example-mcp:2.0.0", + "package:oci:ghcr.io/example/mcp:3.0.0", + "remote:streamable-http:https://api.example.com/mcp", + "remote:sse:https://api.example.com/sse", + ], + ) + + def test_resolves_specific_install_option_by_key(self) -> None: + detail = _detail( + packages=[ + { + "registryType": "npm", + "identifier": "@example/npm-mcp", + "version": "1.0.0", + "transport": {"type": "stdio"}, + }, + { + "registryType": "pypi", + "identifier": "example-mcp", + "version": "2.0.0", + "transport": {"type": "stdio"}, + }, + ], + ) + + option = registry_install_option_by_key(detail, "package:pypi:example-mcp:2.0.0") + spec = resolve_registry_server_spec( + detail, + option_key="package:pypi:example-mcp:2.0.0", + ) + + self.assertIsNotNone(option) + self.assertEqual(spec.command, "uvx") + self.assertEqual(spec.args, ("example-mcp==2.0.0",)) + def test_npm_stdio_package_maps_to_npx_command(self) -> None: spec = resolve_registry_server_spec( _detail( diff --git a/tests/unit/test_mcp_registry_support_policy.py b/tests/unit/test_mcp_registry_support_policy.py new file mode 100644 index 0000000..b6ad44a --- /dev/null +++ b/tests/unit/test_mcp_registry_support_policy.py @@ -0,0 +1,113 @@ +from __future__ import annotations + +import unittest + +from skill_manager.application.mcp.marketplace.support_policy import supported_registry_entry + + +_OFFICIAL_META = "io.modelcontextprotocol.registry/official" + + +def _entry( + *, + packages: list[dict[str, object]] | None = None, + remotes: list[dict[str, object]] | None = None, + latest: bool = True, + status: str = "active", +) -> dict[str, object]: + server: dict[str, object] = { + "name": "ai.example/mcp", + "version": "1.0.0", + "description": "Example", + } + if packages is not None: + server["packages"] = packages + if remotes is not None: + server["remotes"] = remotes + return { + "server": server, + "_meta": { + _OFFICIAL_META: { + "status": status, + "isLatest": latest, + } + }, + } + + +class RegistrySupportPolicyTests(unittest.TestCase): + def test_accepts_supported_local_package(self) -> None: + result = supported_registry_entry( + _entry( + packages=[ + { + "registryType": "npm", + "identifier": "@example/mcp", + "version": "1.0.0", + "transport": {"type": "stdio"}, + } + ] + ) + ) + + self.assertIsNotNone(result) + assert result is not None + _server, options = result + self.assertEqual(options[0].option_key, "package:npm:@example/mcp:1.0.0") + + def test_rejects_unsupported_connection_shapes_generically(self) -> None: + self.assertIsNone( + supported_registry_entry( + _entry(remotes=[{"type": "websocket", "url": "wss://example.com/mcp"}]) + ) + ) + self.assertIsNone( + supported_registry_entry( + _entry( + packages=[ + { + "registryType": "gem", + "identifier": "example-mcp", + "version": "1.0.0", + "transport": {"type": "stdio"}, + } + ] + ) + ) + ) + + def test_rejects_non_current_entries(self) -> None: + self.assertIsNone( + supported_registry_entry( + _entry( + packages=[ + { + "registryType": "npm", + "identifier": "@example/mcp", + "version": "1.0.0", + "transport": {"type": "stdio"}, + } + ], + latest=False, + ) + ) + ) + self.assertIsNone( + supported_registry_entry( + _entry( + packages=[ + { + "registryType": "npm", + "identifier": "@example/mcp", + "version": "1.0.0", + "transport": {"type": "stdio"}, + } + ], + status="deleted", + ) + ) + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_mcp_store.py b/tests/unit/test_mcp_store.py index d0e2bf3..79c82cd 100644 --- a/tests/unit/test_mcp_store.py +++ b/tests/unit/test_mcp_store.py @@ -5,6 +5,7 @@ from pathlib import Path from tempfile import TemporaryDirectory +from skill_manager.application.mcp.install_intent import ManagedMcpRecord, RegistryInstallIntent from skill_manager.application.mcp.store import McpServerSpec, McpServerStore, McpSource @@ -86,10 +87,29 @@ def test_manifest_is_valid_json(self) -> None: payload = json.loads(manifest_path.read_text(encoding="utf-8")) - self.assertEqual(payload["version"], 5) + self.assertEqual(payload["version"], 6) self.assertEqual(len(payload["servers"]), 1) self.assertEqual(payload["servers"][0]["name"], "exa") + def test_round_trip_registry_install_intent(self) -> None: + with TemporaryDirectory() as tmp: + store = McpServerStore(Path(tmp) / "manifest.json") + intent = RegistryInstallIntent.create( + qualified_name="ai.cueapi/mcp", + option_key="package:npm:@cueapi/mcp:0.1.3", + values={"CUEAPI_API_KEY": "secret"}, + ) + store.upsert_record(ManagedMcpRecord(spec=_spec("cueapi"), install_intent=intent)) + + record = store.get_record("cueapi") + + self.assertIsNotNone(record) + assert record is not None + self.assertEqual(record.install_intent, intent) + payload = json.loads((Path(tmp) / "manifest.json").read_text(encoding="utf-8")) + self.assertEqual(payload["servers"][0]["installIntent"]["kind"], "registry") + self.assertEqual(payload["servers"][0]["installIntent"]["values"]["CUEAPI_API_KEY"], "secret") + def test_round_trip_http_spec_preserves_headers_cleartext(self) -> None: with TemporaryDirectory() as tmp: store = McpServerStore(Path(tmp) / "manifest.json")