refactor(capabilities): simplify labels and connection validation (#5580)

This commit is contained in:
Wenchao An 2026-09-20 15:02:02 +08:00 committed by GitHub
parent 5051709343
commit 075f4a3607
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 78 additions and 60 deletions

View File

@ -54,6 +54,37 @@ class CapabilityAdapter(Protocol):
async def install(self, context: AdapterContext, manifest: PluginManifest, name: str, configuration: dict[str, Any]) -> None: ... async def install(self, context: AdapterContext, manifest: PluginManifest, name: str, configuration: dict[str, Any]) -> None: ...
def validate_mcp_connection(configuration: dict[str, Any]) -> None:
"""Validate the normalized transport definition, not manifest form fields."""
from urllib.parse import urlsplit
transport = configuration.get("type", configuration.get("transport", "stdio"))
if not isinstance(transport, str):
raise HTTPException(422, "Supply a supported MCP transport")
if transport in {"http", "sse"}:
url = configuration.get("url")
valid = False
try:
if isinstance(url, str):
parsed = urlsplit(url)
_ = parsed.port # Validate malformed ports too.
has_http_scheme = parsed.scheme in {"https", "http"}
has_host = bool(parsed.hostname)
has_credentials = parsed.username is not None or parsed.password is not None
has_whitespace = any(c.isspace() for c in url)
valid = has_http_scheme and has_host and not has_credentials and not parsed.fragment and not has_whitespace
except ValueError:
valid = False
if not valid:
raise HTTPException(422, "Supply an HTTP(S) MCP server URL without embedded credentials")
return
if transport != "stdio":
raise HTTPException(422, "Supply a supported MCP transport and its required connection fields")
command = configuration.get("command")
if not isinstance(command, str) or not command.strip():
raise HTTPException(422, "Supply a supported MCP transport and its required connection fields")
class MCPAdapter: class MCPAdapter:
async def list_installations(self, context: AdapterContext) -> list[CapabilityInstallation]: async def list_installations(self, context: AdapterContext) -> list[CapabilityInstallation]:
servers = await asyncio.to_thread(mcp._load_raw_mcp_server_responses) servers = await asyncio.to_thread(mcp._load_raw_mcp_server_responses)
@ -97,26 +128,7 @@ class MCPAdapter:
if not name.strip(): if not name.strip():
raise HTTPException(422, "Installation name is required") raise HTTPException(422, "Installation name is required")
if manifest.adapter == "mcp": if manifest.adapter == "mcp":
# The manifest form is normalized to a transport definition by the validate_mcp_connection(configuration)
# UI. Validate that wire contract, not form-only name/auth fields.
from urllib.parse import urlsplit
transport = configuration.get("type", configuration.get("transport", "stdio"))
if not isinstance(transport, str):
raise HTTPException(422, "Supply a supported MCP transport")
if transport in {"http", "sse"}:
url = configuration.get("url")
try:
parsed = urlsplit(url) if isinstance(url, str) else None
valid = parsed is not None and parsed.scheme in {"https", "http"} and bool(parsed.hostname) and parsed.username is None and parsed.password is None and not parsed.fragment and not any(c.isspace() for c in url)
if parsed is not None:
_ = parsed.port # Validate malformed ports too.
except ValueError:
valid = False
if not valid:
raise HTTPException(422, "Supply an HTTP(S) MCP server URL without embedded credentials")
elif transport != "stdio" or not isinstance(configuration.get("command"), str) or not configuration["command"].strip():
raise HTTPException(422, "Supply a supported MCP transport and its required connection fields")
definition = {**configuration, "capability": {"id": str(uuid4()), "plugin_id": manifest.id, "version": manifest.version}} definition = {**configuration, "capability": {"id": str(uuid4()), "plugin_id": manifest.id, "version": manifest.version}}
try: try:
body = mcp.McpConfigUpdateRequest(mcp_servers={name: mcp.McpServerConfigResponse.model_validate(definition)}) body = mcp.McpConfigUpdateRequest(mcp_servers={name: mcp.McpServerConfigResponse.model_validate(definition)})

View File

@ -99,11 +99,7 @@ function MCPServerList({
const previewEntry = previewEntries[0]; const previewEntry = previewEntries[0];
const previewName = const previewName =
editor?.mode === "edit" ? editor.name : (previewEntry?.[0] ?? ""); editor?.mode === "edit" ? editor.name : (previewEntry?.[0] ?? "");
const previewMetadata = catalogForServer( const previewMetadata = catalogForServer(previewEntry?.[1], definitions);
previewName,
previewEntry?.[1],
definitions,
);
const previewIcon = const previewIcon =
draftIcon === undefined && previewEntry draftIcon === undefined && previewEntry
? readPluginIcon(previewEntry[1]) ? readPluginIcon(previewEntry[1])
@ -276,13 +272,13 @@ function MCPServerList({
...catalog.filter( ...catalog.filter(
(item) => (item) =>
!entries.some( !entries.some(
([name, server]) => ([, server]) =>
catalogForServer(name, server, definitions)?.id === item.id, catalogForServer(server, definitions)?.id === item.id,
), ),
), ),
...entries.map(([name, config]): PluginDirectoryEntry => { ...entries.map(([name, config]): PluginDirectoryEntry => {
const displayName = displayServerName(name); const displayName = displayServerName(name);
const metadata = catalogForServer(name, config, definitions); const metadata = catalogForServer(config, definitions);
return { return {
id: `mcp:${name}`, id: `mcp:${name}`,
category: metadata?.category ?? "custom", category: metadata?.category ?? "custom",

View File

@ -15,7 +15,6 @@ export function catalogText(text: LocalizedText, locale: string) {
} }
/** Identity is explicit metadata. A display name never claims an official provider. */ /** Identity is explicit metadata. A display name never claims an official provider. */
export function catalogForServer( export function catalogForServer(
_name: string,
config?: MCPServerConfig, config?: MCPServerConfig,
catalog: PluginManifest[] = [], catalog: PluginManifest[] = [],
) { ) {

View File

@ -20,7 +20,9 @@ import {
installationQuery, installationQuery,
useCapabilityCatalog, useCapabilityCatalog,
} from "@/core/capabilities/hooks"; } from "@/core/capabilities/hooks";
import type { CapabilityInstallation } from "@/core/capabilities/types";
import { useI18n } from "@/core/i18n/hooks"; import { useI18n } from "@/core/i18n/hooks";
import type { Translations } from "@/core/i18n/locales/types";
import { isStaticWebsiteOnly } from "@/core/static-mode"; import { isStaticWebsiteOnly } from "@/core/static-mode";
import { cn } from "@/lib/utils"; import { cn } from "@/lib/utils";
@ -38,6 +40,37 @@ import {
} from "./plugin-directory"; } from "./plugin-directory";
import { PluginIcon } from "./plugin-icon"; import { PluginIcon } from "./plugin-icon";
function getPluginStatusLabel(
adapter: string,
status: CapabilityInstallation | undefined,
unavailable: boolean | undefined,
t: Translations,
labels: ReturnType<typeof capabilityCopy>,
) {
if (unavailable) return labels.adapterError;
if (status) {
if (status.auth_status === "connected") return labels.connected;
if (status.auth_status === "required") return labels.required;
if (status.auth_status === "configured") return labels.configured;
return labels.installed;
}
if (adapter === "guide") return t.capabilities.directory.candidate;
if (adapter === "lark") return t.capabilities.notInstalled;
return labels.notConfigured;
}
function getPluginActionLabel(
adapter: string,
installed: boolean,
canManage: boolean,
t: Translations,
) {
if (installed) return t.capabilities.manage;
if (adapter === "guide" || !canManage) return t.capabilities.directory.view;
if (adapter === "lark") return t.common.install;
return t.capabilities.configure;
}
export function PluginGallery({ query }: { query: string }) { export function PluginGallery({ query }: { query: string }) {
const { t, locale } = useI18n(); const { t, locale } = useI18n();
const copy = t.capabilities.directory; const copy = t.capabilities.directory;
@ -109,23 +142,13 @@ export function PluginGallery({ query }: { query: string }) {
capabilityId={plugin.id} capabilityId={plugin.id}
/> />
} }
label={ label={getPluginStatusLabel(
unavailable plugin.adapter,
? labels.adapterError status,
: status unavailable,
? status.auth_status === "connected" t,
? labels.connected labels,
: status.auth_status === "required" )}
? labels.required
: status.auth_status === "configured"
? labels.configured
: labels.installed
: plugin.adapter === "guide"
? copy.candidate
: plugin.adapter === "lark"
? t.capabilities.notInstalled
: labels.notConfigured
}
onDetails={() => setSelectedId(plugin.id)} onDetails={() => setSelectedId(plugin.id)}
detailsLabel={`${t.capabilities.details} ${catalogText(plugin.name, locale)}`} detailsLabel={`${t.capabilities.details} ${catalogText(plugin.name, locale)}`}
> >
@ -136,15 +159,7 @@ export function PluginGallery({ query }: { query: string }) {
aria-label={`${plugin.adapter === "guide" ? copy.guide : t.capabilities.configure} ${catalogText(plugin.name, locale)}`} aria-label={`${plugin.adapter === "guide" ? copy.guide : t.capabilities.configure} ${catalogText(plugin.name, locale)}`}
onClick={() => setSelectedId(plugin.id)} onClick={() => setSelectedId(plugin.id)}
> >
{status {getPluginActionLabel(plugin.adapter, !!status, canManage, t)}
? t.capabilities.manage
: plugin.adapter === "guide"
? copy.view
: canManage
? plugin.adapter === "lark"
? t.common.install
: t.capabilities.configure
: copy.view}
</Button> </Button>
</PluginRow> </PluginRow>
), ),

View File

@ -30,11 +30,7 @@ export function useCapabilityCatalog() {
}); });
} }
export function useCapabilityInstallations(adapter: string) { export function useCapabilityInstallations(adapter: string) {
return useQuery({ return useQuery(installationQuery(adapter));
queryKey: ["capabilities", "installations", adapter],
queryFn: () =>
request<InstallationList>(`installations/${encodeURIComponent(adapter)}`),
});
} }
export function useInstallCapability() { export function useInstallCapability() {
const client = useQueryClient(); const client = useQueryClient();