From 78cc67c7fe909f0bed43e3229734ea95482847bf Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Wed, 27 May 2026 13:38:03 +0530 Subject: [PATCH] fix(a2a): address PR review comments UI: - Auto-trigger discovery when connection details are filled; remove the "Use these selections" button (selection syncs live to parent, user just clicks Next). - Edit Settings: auto-discover upstream card on open; cross-check with DB-stored card so only already-saved skills/capabilities are pre-ticked. - Extract shared buildDiscoveryRequest + selectionsFromSavedAgentCard helpers into agent_discovery_utils.ts so both add and edit flows share the same logic. Backend: - agent_card.py: rename the proxy security requirements field from the non-standard ``securityRequirements`` to the spec-correct ``security`` key (matches AgentCard TypedDict and A2A/OpenAPI convention). - agent_card.py: remove ``securityRequirements`` from _ALLOWED_TOP_LEVEL_KEYS. - endpoints.py: _build_merged_agent_card now forwards agent_name and description from the request so the stored card reflects the admin- supplied name, not just whatever the upstream card advertised. - utils.py: remove overly-broad ``or "parts" in result`` fallback; use ``kind == "message"`` check only to avoid false matches on future result types that happen to include a ``parts`` field. - test_agent_card.py: update assertions to expect ``security`` key. Co-authored-by: Cursor --- litellm/a2a_protocol/utils.py | 6 +- litellm/proxy/a2a/agent_card.py | 10 +- litellm/proxy/agent_endpoints/endpoints.py | 10 ++ .../test_litellm/proxy/a2a/test_agent_card.py | 5 +- .../src/components/agents/add_agent_form.tsx | 70 ++------- .../agents/agent_card_discovery.test.tsx | 118 ++++++++++---- .../agents/agent_card_discovery.tsx | 148 +++++++++++------- .../agents/agent_discovery_utils.test.ts | 64 ++++++++ .../agents/agent_discovery_utils.ts | 140 +++++++++++++++++ .../src/components/agents/agent_info.tsx | 115 +++++++++++++- 10 files changed, 531 insertions(+), 155 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/agents/agent_discovery_utils.test.ts create mode 100644 ui/litellm-dashboard/src/components/agents/agent_discovery_utils.ts diff --git a/litellm/a2a_protocol/utils.py b/litellm/a2a_protocol/utils.py index 467c4d1f9cc..0dbd1eefc63 100644 --- a/litellm/a2a_protocol/utils.py +++ b/litellm/a2a_protocol/utils.py @@ -60,8 +60,10 @@ class A2ARequestUtils: if not isinstance(result, dict): return "" - # Direct message format (A2A spec) - if result.get("kind") == "message" or "parts" in result: + # Direct message format (A2A spec): detect by explicit kind tag only. + # The "parts" heuristic is too broad and would match any future result + # type that happens to include a "parts" field. + if result.get("kind") == "message": return A2ARequestUtils.extract_text_from_message(result) message = result.get("message", {}) diff --git a/litellm/proxy/a2a/agent_card.py b/litellm/proxy/a2a/agent_card.py index 6cc9670bc09..2e485c6af9a 100644 --- a/litellm/proxy/a2a/agent_card.py +++ b/litellm/proxy/a2a/agent_card.py @@ -59,7 +59,6 @@ _ALLOWED_TOP_LEVEL_KEYS = { "provider", "documentationUrl", "securitySchemes", - "securityRequirements", "security", "supportsAuthenticatedExtendedCard", "signatures", @@ -160,9 +159,10 @@ def merge_agent_card( ] base["securitySchemes"] = deepcopy(LITELLM_SECURITY_SCHEMES) - base["securityRequirements"] = deepcopy(LITELLM_SECURITY_REQUIREMENTS) - # Drop the upstream's per-call ``security`` selector — the proxy enforces - # its own scheme regardless of what upstream required. - base.pop("security", None) + # Use the standard A2A/OpenAPI ``security`` field for requirements, not + # the non-standard ``securityRequirements`` alias. The upstream's own + # ``security`` selector is overwritten here because the proxy enforces its + # own scheme regardless of what upstream required. + base["security"] = deepcopy(LITELLM_SECURITY_REQUIREMENTS) return {key: value for key, value in base.items() if key in _ALLOWED_TOP_LEVEL_KEYS} diff --git a/litellm/proxy/agent_endpoints/endpoints.py b/litellm/proxy/agent_endpoints/endpoints.py index eb88d56f074..a827acfc1c1 100644 --- a/litellm/proxy/agent_endpoints/endpoints.py +++ b/litellm/proxy/agent_endpoints/endpoints.py @@ -48,6 +48,8 @@ def _build_merged_agent_card( *, agent_id: str, http_request: Request, + agent_name: Optional[str] = None, + description: Optional[str] = None, ) -> Dict[str, Any]: """Apply the LiteLLM-fronting merge to ``upstream_card`` for ``agent_id``.""" proxy_base = _proxy_base_url(http_request) @@ -55,6 +57,8 @@ def _build_merged_agent_card( upstream_card, proxy_url=f"{proxy_base}/a2a/{agent_id}", proxy_base_url=proxy_base, + name=agent_name, + description=description, ) @@ -378,6 +382,8 @@ async def create_agent( request.get("agent_card_params"), agent_id=new_agent_id, http_request=http_request, + agent_name=request.get("agent_name"), + description=(request.get("agent_card_params") or {}).get("description"), ) merged_request: AgentConfig = {**request, "agent_card_params": merged_card} # type: ignore[typeddict-item] @@ -580,6 +586,8 @@ async def update_agent( request.get("agent_card_params"), agent_id=agent_id, http_request=http_request, + agent_name=request.get("agent_name"), + description=(request.get("agent_card_params") or {}).get("description"), ) merged_request: AgentConfig = {**request, "agent_card_params": merged_card} # type: ignore[typeddict-item] @@ -686,6 +694,8 @@ async def patch_agent( request.get("agent_card_params"), agent_id=agent_id, http_request=http_request, + agent_name=request.get("agent_name"), + description=(request.get("agent_card_params") or {}).get("description"), ) patch_payload = {**request, "agent_card_params": merged_card} # type: ignore[typeddict-item] diff --git a/tests/test_litellm/proxy/a2a/test_agent_card.py b/tests/test_litellm/proxy/a2a/test_agent_card.py index 46961cf21f9..34183dbbcb2 100644 --- a/tests/test_litellm/proxy/a2a/test_agent_card.py +++ b/tests/test_litellm/proxy/a2a/test_agent_card.py @@ -30,7 +30,6 @@ def _full_upstream_card() -> dict: "defaultInputModes": ["text", "audio"], "defaultOutputModes": ["text"], "securitySchemes": {"upstreamKey": {"type": "apiKey"}}, - "securityRequirements": [{"upstreamKey": []}], "security": [{"upstreamKey": []}], "provider": {"organization": "UpstreamCo", "url": "https://upstream.example"}, "iconUrl": "https://upstream.example/icon.png", @@ -97,8 +96,8 @@ def test_replaces_security_schemes_and_requirements(): _full_upstream_card(), proxy_url=PROXY_URL, proxy_base_url=PROXY_BASE ) assert merged["securitySchemes"] == LITELLM_SECURITY_SCHEMES - assert merged["securityRequirements"] == LITELLM_SECURITY_REQUIREMENTS - assert "security" not in merged + assert merged["security"] == LITELLM_SECURITY_REQUIREMENTS + assert "securityRequirements" not in merged def test_emits_supported_interfaces_pointing_at_proxy(): diff --git a/ui/litellm-dashboard/src/components/agents/add_agent_form.tsx b/ui/litellm-dashboard/src/components/agents/add_agent_form.tsx index a9d2f6f5b0d..7a54ab4fd2f 100644 --- a/ui/litellm-dashboard/src/components/agents/add_agent_form.tsx +++ b/ui/litellm-dashboard/src/components/agents/add_agent_form.tsx @@ -21,8 +21,8 @@ import TeamDropdown from "../common_components/team_dropdown"; import AgentFormFields from "./agent_form_fields"; import AgentCardDiscovery, { DiscoveredAgentCardSelection, - DiscoveryRequestPlan, } from "./agent_card_discovery"; +import { buildDiscoveryRequest } from "./agent_discovery_utils"; import DynamicAgentFormFields, { buildDynamicAgentData } from "./dynamic_agent_form_fields"; import { getDefaultFormValues, buildAgentDataFromForm } from "./agent_config"; import MCPServerSelector from "../mcp_server_management/MCPServerSelector"; @@ -79,10 +79,9 @@ const AddAgentForm: React.FC = ({ const [maxIterations, setMaxIterations] = useState(null); const [maxBudgetPerSession, setMaxBudgetPerSession] = useState(null); - // Last discovery selection the admin clicked "Use these selections" on. - // Dynamic agent forms (LangGraph, Bedrock, Azure) don't render Form.Items - // for skills/capabilities/modes, so we can't carry these through form state - // — we keep them here and overlay them onto agent_card_params at submit. + // Latest upstream card selection from auto-discovery (skills, capabilities, + // name, description). Dynamic agent forms don't render Form.Items for those + // fields, so we overlay this onto agent_card_params at submit. const [appliedDiscoveredSelection, setAppliedDiscoveredSelection] = useState(null); @@ -182,53 +181,15 @@ const AddAgentForm: React.FC = ({ // // Returns undefined when nothing usable is filled in yet, which causes the // component to fall back to a manual URL input. - const discoveryRequest: DiscoveryRequestPlan | undefined = React.useMemo(() => { - const values = watchedFormValues || {}; - - const trim = (v: unknown) => (v ?? "").toString().trim(); - const stripTrailingSlash = (s: string) => s.replace(/\/+$/, ""); - - // LangGraph Platform: base URL is `api_base`, assistant is in - // `assistant_id`. We hit ``{base}/.well-known/agent-card.json?assistant_id=…``. - if (agentType === "langgraph") { - const base = stripTrailingSlash(trim(values.api_base)); - const assistantId = trim(values.assistant_id); - if (!base || !assistantId) return undefined; - const query = `?assistant_id=${encodeURIComponent(assistantId)}`; - return { - url: base, - discovery_mode: "langgraph_platform", - params: { assistant_id: assistantId }, - display_url: `${base}/.well-known/agent-card.json${query}`, - }; - } - - // Pure A2A / use_a2a_form_fields: base URL is in the ``url`` form field. - if (agentType === "a2a" || selectedAgentTypeInfo?.use_a2a_form_fields) { - const base = stripTrailingSlash(trim(values.url)); - if (!base) return undefined; - return { - url: base, - discovery_mode: "well_known_fallback", - display_url: `${base}/.well-known/agent-card.json`, - }; - } - - // Other dynamic types — try to derive a base URL from a credential field - // matching our URL-shaped regex; let the proxy walk the well-known paths. - const credentialFields = selectedAgentTypeInfo?.credential_fields ?? []; - const baseKey = credentialFields.find((f) => - /(^|_)(url|api_base|endpoint)$/i.test(f.key), - )?.key; - if (!baseKey) return undefined; - const base = stripTrailingSlash(trim(values[baseKey])); - if (!base) return undefined; - return { - url: base, - discovery_mode: "well_known_fallback", - display_url: `${base}/.well-known/agent-card.json`, - }; - }, [watchedFormValues, selectedAgentTypeInfo, agentType]); + const discoveryRequest = React.useMemo( + () => + buildDiscoveryRequest( + agentType, + watchedFormValues || {}, + selectedAgentTypeInfo, + ), + [watchedFormValues, selectedAgentTypeInfo, agentType], + ); const handleNext = async () => { try { @@ -686,8 +647,11 @@ const AddAgentForm: React.FC = ({ // every field below; LangGraph and other dynamic forms only pick up the // shared ones (`agent_name`, `description`, plus any credential field whose // key looks URL-ish). - const handleApplyDiscoveredCard = (selection: DiscoveredAgentCardSelection) => { + const handleApplyDiscoveredCard = ( + selection: DiscoveredAgentCardSelection | null, + ) => { setAppliedDiscoveredSelection(selection); + if (!selection) return; const { selected_card, upstream_url } = selection; const skills = (selected_card.skills ?? []).map((s) => ({ id: s.id ?? "", diff --git a/ui/litellm-dashboard/src/components/agents/agent_card_discovery.test.tsx b/ui/litellm-dashboard/src/components/agents/agent_card_discovery.test.tsx index 811f19a20a7..18b874c95ee 100644 --- a/ui/litellm-dashboard/src/components/agents/agent_card_discovery.test.tsx +++ b/ui/litellm-dashboard/src/components/agents/agent_card_discovery.test.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { renderWithProviders } from "../../../tests/test-utils"; @@ -43,10 +43,20 @@ const sampleCard = { describe("AgentCardDiscovery", () => { beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }); mockDiscover.mockReset(); }); - it("renders the URL input and a Discover button", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("renders the URL input and a Re-discover button after manual entry", async () => { + mockDiscover.mockResolvedValue({ + url: "https://upstream.example.com", + agent_card: sampleCard, + }); + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); renderWithProviders( , ); @@ -54,11 +64,21 @@ describe("AgentCardDiscovery", () => { expect( screen.getByPlaceholderText("https://upstream-agent.example.com"), ).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /discover/i })).toBeInTheDocument(); + + await user.type( + screen.getByPlaceholderText("https://upstream-agent.example.com"), + "https://upstream.example.com", + ); + await vi.advanceTimersByTimeAsync(500); + + await waitFor(() => expect(mockDiscover).toHaveBeenCalled()); + expect( + await screen.findByRole("button", { name: /re-discover/i }), + ).toBeInTheDocument(); }); - it("shows an error when discover is clicked without a URL", async () => { - const user = userEvent.setup(); + it("shows an error when re-discover is clicked without a URL", async () => { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); renderWithProviders( , ); @@ -70,12 +90,12 @@ describe("AgentCardDiscovery", () => { expect(mockDiscover).not.toHaveBeenCalled(); }); - it("renders the upstream skills and capabilities on success", async () => { + it("auto-discovers and renders upstream skills on success", async () => { mockDiscover.mockResolvedValueOnce({ url: "https://upstream.example.com", agent_card: sampleCard, }); - const user = userEvent.setup(); + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); renderWithProviders( , ); @@ -84,19 +104,21 @@ describe("AgentCardDiscovery", () => { screen.getByPlaceholderText("https://upstream-agent.example.com"), "https://upstream.example.com", ); - await user.click(screen.getByRole("button", { name: /discover/i })); + await vi.advanceTimersByTimeAsync(500); expect(await screen.findByText("Upstream card loaded")).toBeInTheDocument(); expect(screen.getByText("Search")).toBeInTheDocument(); expect(screen.getByText("Summarize")).toBeInTheDocument(); - // Only proxy-supported capabilities surface (streaming). expect(screen.getByText(/^streaming$/i)).toBeInTheDocument(); expect(screen.queryByText(/pushNotifications/i)).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /use these selections/i }), + ).not.toBeInTheDocument(); }); it("shows an inline error when discovery fails", async () => { mockDiscover.mockRejectedValueOnce(new Error("upstream unreachable")); - const user = userEvent.setup(); + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); renderWithProviders( , ); @@ -105,19 +127,19 @@ describe("AgentCardDiscovery", () => { screen.getByPlaceholderText("https://upstream-agent.example.com"), "https://nope.example", ); - await user.click(screen.getByRole("button", { name: /discover/i })); + await vi.advanceTimersByTimeAsync(500); expect(await screen.findByText("Discovery failed")).toBeInTheDocument(); expect(screen.getByText(/upstream unreachable/)).toBeInTheDocument(); }); - it("emits the selected subset when the user applies the card", async () => { + it("syncs the selected subset to the parent as the user edits", async () => { mockDiscover.mockResolvedValueOnce({ url: "https://upstream.example.com", agent_card: sampleCard, }); const onApply = vi.fn(); - const user = userEvent.setup(); + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); renderWithProviders( , ); @@ -126,10 +148,14 @@ describe("AgentCardDiscovery", () => { screen.getByPlaceholderText("https://upstream-agent.example.com"), "https://upstream.example.com", ); - await user.click(screen.getByRole("button", { name: /discover/i })); + await vi.advanceTimersByTimeAsync(500); await screen.findByText("Upstream card loaded"); - // Deselect the "Summarize" skill by clicking its row's checkbox. + await waitFor(() => expect(onApply).toHaveBeenCalled()); + const initialSelection = onApply.mock.calls.at(-1)?.[0]; + expect(initialSelection.upstream_url).toBe("https://upstream.example.com"); + expect(initialSelection.selected_card.skills).toHaveLength(2); + const summarizeLabel = screen.getByText("Summarize").closest("label"); expect(summarizeLabel).toBeTruthy(); const summarizeCheckbox = summarizeLabel!.querySelector( @@ -137,15 +163,11 @@ describe("AgentCardDiscovery", () => { ) as HTMLInputElement; await user.click(summarizeCheckbox); - await user.click(screen.getByRole("button", { name: /use these selections/i })); - - await waitFor(() => expect(onApply).toHaveBeenCalledTimes(1)); - const selection = onApply.mock.calls[0][0]; - expect(selection.upstream_url).toBe("https://upstream.example.com"); - expect(selection.raw_card).toEqual(sampleCard); - expect(selection.selected_card.skills).toHaveLength(1); - expect(selection.selected_card.skills[0].id).toBe("search"); - expect(selection.selected_card.name).toBe("Upstream Agent"); + await waitFor(() => { + const latest = onApply.mock.calls.at(-1)?.[0]; + expect(latest.selected_card.skills).toHaveLength(1); + expect(latest.selected_card.skills[0].id).toBe("search"); + }); }); it("hides the URL input and shows the display URL when parent-driven", () => { @@ -163,11 +185,9 @@ describe("AgentCardDiscovery", () => { />, ); - // Free-form URL input is gone. expect( screen.queryByPlaceholderText("https://upstream-agent.example.com"), ).not.toBeInTheDocument(); - // The exact URL the proxy will hit is visible. expect( screen.getByText( "http://localhost:2024/.well-known/agent-card.json?assistant_id=agent", @@ -175,12 +195,11 @@ describe("AgentCardDiscovery", () => { ).toBeInTheDocument(); }); - it("forwards discovery_mode and params from the parent plan", async () => { + it("auto-discovers with discovery_mode and params from the parent plan", async () => { mockDiscover.mockResolvedValueOnce({ url: "http://localhost:2024", agent_card: sampleCard, }); - const user = userEvent.setup(); renderWithProviders( { />, ); - await user.click(screen.getByRole("button", { name: /discover/i })); - + await vi.advanceTimersByTimeAsync(0); await waitFor(() => expect(mockDiscover).toHaveBeenCalledTimes(1)); expect(mockDiscover).toHaveBeenCalledWith("tok", "http://localhost:2024", { discovery_mode: "langgraph_platform", @@ -204,7 +222,7 @@ describe("AgentCardDiscovery", () => { }); }); - it("disables Discover until the parent provides a usable URL", async () => { + it("disables Re-discover until the parent provides a usable URL", async () => { renderWithProviders( { name: /discover/i, }) as HTMLButtonElement).disabled, ).toBe(true); + expect(mockDiscover).not.toHaveBeenCalled(); + }); + + it("pre-selects only skills present in savedAgentCard when editing", async () => { + mockDiscover.mockResolvedValueOnce({ + url: "http://localhost:2024", + agent_card: sampleCard, + }); + const onApply = vi.fn(); + renderWithProviders( + , + ); + + await vi.advanceTimersByTimeAsync(0); + await screen.findByText("Upstream card loaded"); + + await waitFor(() => expect(onApply).toHaveBeenCalled()); + const selection = onApply.mock.calls.at(-1)?.[0]; + expect(selection.selected_card.skills).toHaveLength(1); + expect(selection.selected_card.skills[0].id).toBe("search"); + expect(selection.selected_card.name).toBe("DB Agent"); + expect(selection.selected_card.capabilities.streaming).toBe(false); }); it("blocks discover when no access token is provided", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); renderWithProviders( , ); diff --git a/ui/litellm-dashboard/src/components/agents/agent_card_discovery.tsx b/ui/litellm-dashboard/src/components/agents/agent_card_discovery.tsx index b21f2999bb6..5505c9035ca 100644 --- a/ui/litellm-dashboard/src/components/agents/agent_card_discovery.tsx +++ b/ui/litellm-dashboard/src/components/agents/agent_card_discovery.tsx @@ -1,6 +1,6 @@ "use client"; -import React, { useState } from "react"; +import React, { useCallback, useEffect, useRef, useState } from "react"; import { Alert, Button, @@ -26,9 +26,13 @@ import { import { DiscoveredAgentCard, - DiscoveryMode, discoverAgentCardCall, } from "../networking"; +import { + selectionsFromSavedAgentCard, + selectionsFromUpstreamCard, + skillId, +} from "./agent_discovery_utils"; const { Text, Paragraph } = Typography; const { Panel } = Collapse; @@ -44,27 +48,14 @@ export interface DiscoveredAgentCardSelection { upstream_url: string; } -/** - * What the parent wants the discovery endpoint to do. When the parent can - * derive this from form state (e.g. agent type = LangGraph + assistant_id + - * api_base), it owns the values; the component just relays them. Different - * upstreams use different URL conventions, so the mode matters. - */ -export interface DiscoveryRequestPlan { - /** Base URL to send to the proxy. */ - url: string; - /** Which dispatch path the proxy should use. */ - discovery_mode: DiscoveryMode; - /** Mode-specific params (e.g. ``{assistant_id}`` for LangGraph). */ - params?: Record; - /** Human-readable rendering of the URL the proxy will ultimately fetch. - * Shown in the UI so the admin can see what we'll hit. */ - display_url?: string; -} +export type { DiscoveryRequestPlan } from "./agent_discovery_utils"; +import type { DiscoveryRequestPlan } from "./agent_discovery_utils"; interface AgentCardDiscoveryProps { accessToken: string | null; - onApply: (selection: DiscoveredAgentCardSelection) => void; + /** Called whenever the upstream card or the user's selections change. Pass + * ``null`` when discovery is cleared or fails so the parent can reset. */ + onApply: (selection: DiscoveredAgentCardSelection | null) => void; /** * Parent-supplied discovery plan. When provided the component uses these * values verbatim and hides its free-form URL input — the parent is the @@ -73,32 +64,19 @@ interface AgentCardDiscoveryProps { * input that defaults to ``well_known_fallback`` mode. */ discoveryRequest?: DiscoveryRequestPlan; + /** When editing an existing agent, the card stored in the DB. Upstream + * discovery lists everything available; only skills/capabilities present + * here are pre-selected. */ + savedAgentCard?: DiscoveredAgentCard | null; } const ALLOWED_CAPABILITY_KEYS = ["streaming"] as const; -const skillId = (skill: any, idx: number): string => - skill?.id ?? skill?.name ?? `skill-${idx}`; - -/** - * Mirrors the proxy-side `_ALLOWED_CAPABILITY_KEYS` allowlist. Keep these in - * sync — anything we surface here that the proxy strips would look like a - * silent drop to the admin. - */ -const filterCapabilitiesForUI = ( - capabilities: Record | undefined, -): Record => { - if (!capabilities) return {}; - return ALLOWED_CAPABILITY_KEYS.reduce>((acc, key) => { - if (key in capabilities) acc[key] = Boolean(capabilities[key]); - return acc; - }, {}); -}; - const AgentCardDiscovery: React.FC = ({ accessToken, onApply, discoveryRequest, + savedAgentCard, }) => { // When the parent drives discovery, ``manualUrl`` is unused — the URL // comes from ``discoveryRequest.url`` directly. When the parent hasn't @@ -119,16 +97,24 @@ const AgentCardDiscovery: React.FC = ({ >({}); const resetSelections = (fresh: DiscoveredAgentCard) => { - setEditedName(fresh.name ?? ""); - setEditedDescription(fresh.description ?? ""); - const skills = fresh.skills ?? []; - setSelectedSkillIds(new Set(skills.map((s, i) => skillId(s, i)))); - setSelectedCapabilities(filterCapabilitiesForUI(fresh.capabilities)); + const initial = savedAgentCard + ? selectionsFromSavedAgentCard(fresh, savedAgentCard) + : selectionsFromUpstreamCard(fresh); + setEditedName(initial.editedName); + setEditedDescription(initial.editedDescription); + setSelectedSkillIds(initial.selectedSkillIds); + setSelectedCapabilities(initial.selectedCapabilities); }; - const handleDiscover = async () => { + const onApplyRef = useRef(onApply); + onApplyRef.current = onApply; + const discoverRequestIdRef = useRef(0); + const lastSyncedSelectionRef = useRef(null); + + const handleDiscover = useCallback(async () => { if (!accessToken) { setError("No access token available"); + onApplyRef.current(null); return; } const trimmed = effectiveUrl.trim(); @@ -138,9 +124,12 @@ const AgentCardDiscovery: React.FC = ({ ? "Fill in the agent's connection details above first" : "Enter the agent's base URL first", ); + setCard(null); + onApplyRef.current(null); return; } + const requestId = ++discoverRequestIdRef.current; setLoading(true); setError(null); try { @@ -154,15 +143,49 @@ const AgentCardDiscovery: React.FC = ({ } : undefined, ); + if (requestId !== discoverRequestIdRef.current) return; + lastSyncedSelectionRef.current = null; setCard(response.agent_card); resetSelections(response.agent_card); } catch (e: any) { + if (requestId !== discoverRequestIdRef.current) return; setError(e?.message ? String(e.message) : "Failed to discover agent card"); setCard(null); + lastSyncedSelectionRef.current = null; + onApplyRef.current(null); } finally { - setLoading(false); + if (requestId === discoverRequestIdRef.current) { + setLoading(false); + } } - }; + }, [accessToken, discoveryRequest, effectiveUrl, isParentDriven]); + + // Auto-discover when the URL (or parent plan) becomes available. + useEffect(() => { + if (!accessToken) return; + const trimmed = effectiveUrl.trim(); + if (!trimmed) { + setCard(null); + setError(null); + lastSyncedSelectionRef.current = null; + onApplyRef.current(null); + return; + } + + const debounceMs = isParentDriven ? 0 : 400; + const timer = window.setTimeout(() => { + void handleDiscover(); + }, debounceMs); + return () => window.clearTimeout(timer); + }, [ + accessToken, + effectiveUrl, + isParentDriven, + discoveryRequest?.url, + discoveryRequest?.discovery_mode, + discoveryRequest?.params, + handleDiscover, + ]); const toggleSkill = (id: string, checked: boolean) => { setSelectedSkillIds((prev) => { @@ -173,8 +196,8 @@ const AgentCardDiscovery: React.FC = ({ }); }; - const handleApply = () => { - if (!card) return; + const buildSelection = useCallback((): DiscoveredAgentCardSelection | null => { + if (!card) return null; const skills = card.skills ?? []; const filteredSkills = skills.filter((s, i) => selectedSkillIds.has(skillId(s, i)), @@ -188,12 +211,30 @@ const AgentCardDiscovery: React.FC = ({ capabilities: { ...selectedCapabilities }, }; - onApply({ + return { raw_card: card, selected_card, upstream_url: effectiveUrl.trim(), - }); - }; + }; + }, [ + card, + editedDescription, + editedName, + effectiveUrl, + selectedCapabilities, + selectedSkillIds, + ]); + + // Keep the parent form in sync as the user edits selections — no extra + // "apply" click needed before hitting Next. + useEffect(() => { + if (!card) return; + const selection = buildSelection(); + const serialized = JSON.stringify(selection); + if (lastSyncedSelectionRef.current === serialized) return; + lastSyncedSelectionRef.current = serialized; + onApplyRef.current(selection); + }, [buildSelection, card]); const skillCount = card?.skills?.length ?? 0; const selectedSkillCount = selectedSkillIds.size; @@ -428,11 +469,6 @@ const AgentCardDiscovery: React.FC = ({ -
- -
)} diff --git a/ui/litellm-dashboard/src/components/agents/agent_discovery_utils.test.ts b/ui/litellm-dashboard/src/components/agents/agent_discovery_utils.test.ts new file mode 100644 index 00000000000..632fba1532c --- /dev/null +++ b/ui/litellm-dashboard/src/components/agents/agent_discovery_utils.test.ts @@ -0,0 +1,64 @@ +import { describe, it, expect } from "vitest"; +import { + selectionsFromSavedAgentCard, + selectionsFromUpstreamCard, + skillId, +} from "./agent_discovery_utils"; + +const upstreamCard = { + name: "Upstream Agent", + description: "Upstream description", + capabilities: { streaming: true }, + skills: [ + { id: "search", name: "Search", description: "Search the web" }, + { id: "summarize", name: "Summarize", description: "Summarize docs" }, + { id: "chat", name: "Chat", description: "General chat" }, + ], +}; + +describe("selectionsFromSavedAgentCard", () => { + it("pre-selects only skills that exist in the saved DB card", () => { + const savedCard = { + name: "My Agent", + description: "Saved description", + capabilities: { streaming: false }, + skills: [{ id: "search", name: "Search" }], + }; + + const result = selectionsFromSavedAgentCard(upstreamCard, savedCard); + + expect(result.editedName).toBe("My Agent"); + expect(result.editedDescription).toBe("Saved description"); + expect(result.selectedCapabilities.streaming).toBe(false); + expect(result.selectedSkillIds.has(skillId(upstreamCard.skills![0], 0))).toBe( + true, + ); + expect( + result.selectedSkillIds.has(skillId(upstreamCard.skills![1], 1)), + ).toBe(false); + expect( + result.selectedSkillIds.has(skillId(upstreamCard.skills![2], 2)), + ).toBe(false); + }); + + it("matches saved skills by name when id is missing", () => { + const savedCard = { + skills: [{ name: "Summarize" }], + }; + + const result = selectionsFromSavedAgentCard(upstreamCard, savedCard); + + expect( + result.selectedSkillIds.has(skillId(upstreamCard.skills![1], 1)), + ).toBe(true); + expect(result.selectedSkillIds.size).toBe(1); + }); +}); + +describe("selectionsFromUpstreamCard", () => { + it("selects all upstream skills for create flow", () => { + const result = selectionsFromUpstreamCard(upstreamCard); + expect(result.selectedSkillIds.size).toBe(3); + expect(result.editedName).toBe("Upstream Agent"); + }); +}); diff --git a/ui/litellm-dashboard/src/components/agents/agent_discovery_utils.ts b/ui/litellm-dashboard/src/components/agents/agent_discovery_utils.ts new file mode 100644 index 00000000000..8ce4332dbb3 --- /dev/null +++ b/ui/litellm-dashboard/src/components/agents/agent_discovery_utils.ts @@ -0,0 +1,140 @@ +import { + AgentCreateInfo, + DiscoveredAgentCard, + DiscoveryMode, +} from "../networking"; + +export interface DiscoveryRequestPlan { + url: string; + discovery_mode: DiscoveryMode; + params?: Record; + display_url?: string; +} + +export const skillId = (skill: any, idx: number): string => + skill?.id ?? skill?.name ?? `skill-${idx}`; + +const ALLOWED_CAPABILITY_KEYS = ["streaming"] as const; + +export const filterCapabilitiesForUI = ( + capabilities: Record | undefined, +): Record => { + if (!capabilities) return {}; + return ALLOWED_CAPABILITY_KEYS.reduce>((acc, key) => { + if (key in capabilities) acc[key] = Boolean(capabilities[key]); + return acc; + }, {}); +}; + +/** + * After fetching the full upstream card, pre-select only skills and + * capabilities that already exist on the agent record in the DB. + */ +export const selectionsFromSavedAgentCard = ( + upstreamCard: DiscoveredAgentCard, + savedCard: DiscoveredAgentCard | undefined | null, +): { + editedName: string; + editedDescription: string; + selectedSkillIds: Set; + selectedCapabilities: Record; +} => { + const upstreamSkills = upstreamCard.skills ?? []; + const savedSkills = savedCard?.skills ?? []; + + const savedSkillIds = new Set( + savedSkills.map((s) => s?.id).filter(Boolean) as string[], + ); + const savedSkillNames = new Set( + savedSkills.map((s) => s?.name).filter(Boolean) as string[], + ); + + const selectedSkillIds = new Set(); + upstreamSkills.forEach((skill, idx) => { + const id = skillId(skill, idx); + const matchesById = skill.id && savedSkillIds.has(skill.id); + const matchesByName = skill.name && savedSkillNames.has(skill.name); + if (matchesById || matchesByName) { + selectedSkillIds.add(id); + } + }); + + const selectedCapabilities = filterCapabilitiesForUI(upstreamCard.capabilities); + if (savedCard?.capabilities) { + for (const key of ALLOWED_CAPABILITY_KEYS) { + if (key in savedCard.capabilities) { + selectedCapabilities[key] = Boolean(savedCard.capabilities[key]); + } + } + } + + return { + editedName: savedCard?.name ?? upstreamCard.name ?? "", + editedDescription: savedCard?.description ?? upstreamCard.description ?? "", + selectedSkillIds, + selectedCapabilities, + }; +}; + +/** Default for create flow: select everything the upstream advertises. */ +export const selectionsFromUpstreamCard = ( + upstreamCard: DiscoveredAgentCard, +): { + editedName: string; + editedDescription: string; + selectedSkillIds: Set; + selectedCapabilities: Record; +} => { + const upstreamSkills = upstreamCard.skills ?? []; + return { + editedName: upstreamCard.name ?? "", + editedDescription: upstreamCard.description ?? "", + selectedSkillIds: new Set(upstreamSkills.map((s, i) => skillId(s, i))), + selectedCapabilities: filterCapabilitiesForUI(upstreamCard.capabilities), + }; +}; + +export const buildDiscoveryRequest = ( + agentType: string, + values: Record, + selectedAgentTypeInfo?: AgentCreateInfo, +): DiscoveryRequestPlan | undefined => { + const trim = (v: unknown) => (v ?? "").toString().trim(); + const stripTrailingSlash = (s: string) => s.replace(/\/+$/, ""); + + if (agentType === "langgraph") { + const base = stripTrailingSlash(trim(values.api_base)); + const assistantId = trim(values.assistant_id); + if (!base || !assistantId) return undefined; + const query = `?assistant_id=${encodeURIComponent(assistantId)}`; + return { + url: base, + discovery_mode: "langgraph_platform", + params: { assistant_id: assistantId }, + display_url: `${base}/.well-known/agent-card.json${query}`, + }; + } + + if (agentType === "a2a" || selectedAgentTypeInfo?.use_a2a_form_fields) { + const base = stripTrailingSlash(trim(values.url)); + if (!base) return undefined; + return { + url: base, + discovery_mode: "well_known_fallback", + display_url: `${base}/.well-known/agent-card.json`, + }; + } + + const credentialFields = selectedAgentTypeInfo?.credential_fields ?? []; + const baseKey = credentialFields.find((f) => + /(^|_)(url|api_base|endpoint)$/i.test(f.key), + )?.key; + if (!baseKey) return undefined; + const base = stripTrailingSlash(trim(values[baseKey])); + if (!base) return undefined; + return { + url: base, + discovery_mode: "well_known_fallback", + display_url: `${base}/.well-known/agent-card.json`, + }; +}; diff --git a/ui/litellm-dashboard/src/components/agents/agent_info.tsx b/ui/litellm-dashboard/src/components/agents/agent_info.tsx index d543be8356a..ba968921c93 100644 --- a/ui/litellm-dashboard/src/components/agents/agent_info.tsx +++ b/ui/litellm-dashboard/src/components/agents/agent_info.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from "react"; +import React, { useState, useEffect, useMemo } from "react"; import { Card, Title, Text, Button as TremorButton, Tab, TabGroup, TabList, TabPanel, TabPanels} from "@tremor/react"; import { Form, Input, InputNumber, Button as AntButton, Spin, Descriptions, Divider } from "antd"; import MessageManager from "@/components/molecules/message_manager"; @@ -10,6 +10,10 @@ import DynamicAgentFormFields, { buildDynamicAgentData } from "./dynamic_agent_f import { buildAgentDataFromForm, parseAgentForForm } from "./agent_config"; import AgentCostView from "./agent_cost_view"; import { detectAgentType, parseDynamicAgentForForm } from "./agent_type_utils"; +import AgentCardDiscovery, { + DiscoveredAgentCardSelection, +} from "./agent_card_discovery"; +import { buildDiscoveryRequest } from "./agent_discovery_utils"; interface AgentInfoViewProps { agentId: string; @@ -31,6 +35,8 @@ const AgentInfoView: React.FC = ({ const [form] = Form.useForm(); const [agentTypeMetadata, setAgentTypeMetadata] = useState([]); const [detectedAgentType, setDetectedAgentType] = useState("a2a"); + const [appliedDiscoveredSelection, setAppliedDiscoveredSelection] = + useState(null); useEffect(() => { const fetchMetadata = async () => { @@ -93,6 +99,85 @@ const AgentInfoView: React.FC = ({ }, [agentTypeMetadata, agent]); const selectedAgentTypeInfo = agentTypeMetadata.find(t => t.agent_type === detectedAgentType); + const watchedFormValues = Form.useWatch([], form); + + const discoveryRequest = useMemo( + () => + buildDiscoveryRequest( + detectedAgentType, + watchedFormValues || {}, + selectedAgentTypeInfo, + ), + [watchedFormValues, selectedAgentTypeInfo, detectedAgentType], + ); + + const overlayDiscoveredCardParams = ( + agentData: Record, + ): Record => { + if (!appliedDiscoveredSelection) return agentData; + const discovered = appliedDiscoveredSelection.selected_card; + return { + ...agentData, + agent_card_params: { + ...agentData.agent_card_params, + name: discovered.name ?? agentData.agent_card_params?.name, + description: + discovered.description ?? agentData.agent_card_params?.description, + ...(Array.isArray(discovered.skills) && { + skills: discovered.skills, + }), + ...(discovered.capabilities && { + capabilities: discovered.capabilities, + }), + ...(Array.isArray(discovered.defaultInputModes) && + discovered.defaultInputModes.length > 0 && { + defaultInputModes: discovered.defaultInputModes, + }), + ...(Array.isArray(discovered.defaultOutputModes) && + discovered.defaultOutputModes.length > 0 && { + defaultOutputModes: discovered.defaultOutputModes, + }), + ...(discovered.provider && { provider: discovered.provider }), + ...(discovered.iconUrl && { iconUrl: discovered.iconUrl }), + ...(discovered.documentationUrl && { + documentationUrl: discovered.documentationUrl, + }), + }, + }; + }; + + const handleApplyDiscoveredCard = ( + selection: DiscoveredAgentCardSelection | null, + ) => { + setAppliedDiscoveredSelection(selection); + if (!selection) return; + const { selected_card } = selection; + const skills = (selected_card.skills ?? []).map((s) => ({ + id: s.id ?? "", + name: s.name ?? "", + description: s.description ?? "", + tags: s.tags ?? [], + examples: s.examples ?? [], + })); + + const fieldsToSet: Record = { + name: selected_card.name, + description: selected_card.description, + streaming: Boolean(selected_card.capabilities?.streaming), + skills, + iconUrl: selected_card.iconUrl, + documentationUrl: selected_card.documentationUrl, + }; + + const urlCredentialKeys = (selectedAgentTypeInfo?.credential_fields ?? []) + .map((f) => f.key) + .filter((key) => /(^|_)(url|api_base|endpoint)$/i.test(key)); + for (const key of urlCredentialKeys) { + fieldsToSet[key] = selection.upstream_url; + } + + form.setFieldsValue(fieldsToSet); + }; const handleUpdate = async (values: any) => { if (!accessToken || !agent) return; @@ -105,12 +190,15 @@ const AgentInfoView: React.FC = ({ updateData = buildAgentDataFromForm(values, agent); } else if (selectedAgentTypeInfo) { updateData = buildDynamicAgentData(values, selectedAgentTypeInfo); - // Preserve the agent_name from form updateData.agent_name = values.agent_name; } else { updateData = buildAgentDataFromForm(values, agent); } - + + if (appliedDiscoveredSelection) { + updateData = overlayDiscoveredCardParams(updateData); + } + await patchAgentCall(accessToken, agentId, updateData); MessageManager.success("Agent updated successfully"); setIsEditing(false); @@ -278,7 +366,14 @@ const AgentInfoView: React.FC = ({
Agent Settings {!isEditing && ( - setIsEditing(true)}>Edit Settings + { + setAppliedDiscoveredSelection(null); + setIsEditing(true); + }} + > + Edit Settings + )}
@@ -300,6 +395,17 @@ const AgentInfoView: React.FC = ({ )} + {discoveryRequest && ( +
+ +
+ )} + Rate Limits
@@ -321,6 +427,7 @@ const AgentInfoView: React.FC = ({
{ + setAppliedDiscoveredSelection(null); setIsEditing(false); fetchAgentInfo(); }}>