From c61e15f12d2e418eaaaeb0128a65274839175870 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 20 May 2026 13:07:38 -0400 Subject: [PATCH 1/9] Port resource preview UX improvements to PromptsScreen (#1328) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brings the prompt-fetching flow up to parity with the resource preview work from #1326: - Cap the argument-form pane at maw=40% so a bare input + button doesn't stretch across wide displays. - Auto-fetch no-argument prompts the moment they're picked from the sidebar (handleSelectPrompt routes them straight to onGetPrompt({})), and hide the form once the user clicks Get Prompt — the result panel takes the same fixed-height column the resource preview uses. - Apply the PreviewCard variant=preview pattern: card sizes to content but caps at viewport, PromptMessagesDisplay pins its header (flex 0 0 auto) and lets the inner ScrollArea (flex 0 1 auto, mih 0) absorb overflow. - Tag getPromptState with the prompt name in App.tsx so the screen can ignore a stale result whose name no longer matches the selection. - MessageBubble now routes each content block through ContentViewer (markdown for text via mimeType="text/markdown", image/audio/resource via existing ContentViewer branches). Non-renderable block types (tool_use, tool_result) are filtered out; the role-label header keeps the bubble visible regardless. - PromptArgumentsForm gains onCompleteArgument + completionsSupported; when both are set, each input becomes Mantine Autocomplete with the same per-arg AbortController + per-arg debounce timer pattern as ResourceTemplatePanel. PromptsScreen + InspectorView + App.tsx wire the callback to inspectorClient.getCompletions with a ref/prompt envelope. Closes #1328. Co-Authored-By: Claude Opus 4.7 (1M context) --- clients/web/src/App.tsx | 12 +- .../MessageBubble/MessageBubble.test.tsx | 47 +++- .../elements/MessageBubble/MessageBubble.tsx | 109 ++++----- .../PromptArgumentsForm.test.tsx | 105 +++++++++ .../PromptArgumentsForm.tsx | 146 ++++++++++-- .../PromptMessagesDisplay.tsx | 58 ++++- .../SamplingRequestPanel.test.tsx | 2 +- .../PromptsScreen/PromptsScreen.test.tsx | 188 ++++++++++++--- .../screens/PromptsScreen/PromptsScreen.tsx | 217 ++++++++++++------ .../views/InspectorView/InspectorView.tsx | 2 + 10 files changed, 678 insertions(+), 208 deletions(-) diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 8b9784d92..ddcc1f24f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -455,13 +455,21 @@ function App() { const onGetPrompt = useCallback( async (name: string, args: Record) => { if (!inspectorClient) return; - setGetPromptState({ status: "pending" }); + // Tag the in-flight + final state with the prompt name so the + // PromptsScreen can guard against showing a stale result for a + // prompt the user has already navigated away from. + setGetPromptState({ status: "pending", promptName: name }); try { const invocation = await inspectorClient.getPrompt(name, args); - setGetPromptState({ status: "ok", result: invocation.result }); + setGetPromptState({ + status: "ok", + promptName: name, + result: invocation.result, + }); } catch (err) { setGetPromptState({ status: "error", + promptName: name, error: err instanceof Error ? err.message : String(err), }); } diff --git a/clients/web/src/components/elements/MessageBubble/MessageBubble.test.tsx b/clients/web/src/components/elements/MessageBubble/MessageBubble.test.tsx index 0100adec2..ddabf03cd 100644 --- a/clients/web/src/components/elements/MessageBubble/MessageBubble.test.tsx +++ b/clients/web/src/components/elements/MessageBubble/MessageBubble.test.tsx @@ -7,17 +7,29 @@ import { renderWithMantine, screen } from "../../../test/renderWithMantine"; import { MessageBubble } from "./MessageBubble"; describe("MessageBubble", () => { - it("renders a text sampling message", () => { + it("renders a text sampling message as markdown", () => { const message: SamplingMessage = { role: "user", content: { type: "text", text: "hello" }, }; renderWithMantine(); expect(screen.getByText("[0] role: user")).toBeInTheDocument(); - expect(screen.getByText('"hello"')).toBeInTheDocument(); + expect(screen.getByText("hello")).toBeInTheDocument(); }); - it("renders a copy button when there is text", () => { + it("renders markdown formatting in prompt text", () => { + const message: PromptMessage = { + role: "assistant", + content: { type: "text", text: "# Heading\n\nSome **bold** text" }, + }; + renderWithMantine(); + expect( + screen.getByRole("heading", { level: 1, name: "Heading" }), + ).toBeInTheDocument(); + expect(screen.getByText("bold")).toBeInTheDocument(); + }); + + it("renders a copy button for text content via ContentViewer copyable", () => { const message: SamplingMessage = { role: "user", content: { type: "text", text: "hello" }, @@ -52,7 +64,7 @@ describe("MessageBubble", () => { ); }); - it("renders embedded resource text from a prompt message array", () => { + it("renders embedded resource text from a prompt message", () => { const message: PromptMessage = { role: "user", content: { @@ -61,7 +73,7 @@ describe("MessageBubble", () => { }, }; renderWithMantine(); - expect(screen.getByText('"embedded"')).toBeInTheDocument(); + expect(screen.getByText("embedded")).toBeInTheDocument(); }); it("renders blob resource placeholder", () => { @@ -77,24 +89,39 @@ describe("MessageBubble", () => { }, }; renderWithMantine(); - expect(screen.getByText('"[resource: file:///b]"')).toBeInTheDocument(); + expect(screen.getByText("[blob: file:///b]")).toBeInTheDocument(); }); it("renders resource_link content", () => { const message = { role: "user", - content: { type: "resource_link", uri: "ui://app" }, + content: { type: "resource_link", uri: "ui://app", name: "Cool App" }, } as unknown as PromptMessage; renderWithMantine(); - expect(screen.getByText('"[resource: ui://app]"')).toBeInTheDocument(); + expect(screen.getByText("Cool App")).toBeInTheDocument(); }); - it("renders fallback for unknown content types", () => { + it("still renders the role label for unknown content types", () => { const message = { role: "user", content: { type: "weird" }, } as unknown as SamplingMessage; renderWithMantine(); - expect(screen.getByText('"[weird]"')).toBeInTheDocument(); + // ContentViewer returns null for unknown block types; the bubble's + // role-label header still renders so the message isn't invisible. + expect(screen.getByText("[6] role: user")).toBeInTheDocument(); + }); + + it("renders multiple content blocks from an array", () => { + const message: PromptMessage = { + role: "user", + content: [ + { type: "text", text: "first" }, + { type: "text", text: "second" }, + ] as unknown as PromptMessage["content"], + }; + renderWithMantine(); + expect(screen.getByText("first")).toBeInTheDocument(); + expect(screen.getByText("second")).toBeInTheDocument(); }); }); diff --git a/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx b/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx index 951369f93..29647aa1d 100644 --- a/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx +++ b/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx @@ -1,72 +1,47 @@ -import { Group, Image, Paper, Stack, Text } from "@mantine/core"; +import { Group, Paper, Stack, Text } from "@mantine/core"; import type { + ContentBlock, PromptMessage, SamplingMessage, } from "@modelcontextprotocol/sdk/types.js"; -import { CopyButton } from "../CopyButton/CopyButton"; +import { ContentViewer } from "../ContentViewer/ContentViewer"; export interface MessageBubbleProps { index: number; message: SamplingMessage | PromptMessage; } -function buildDataUri(mimeType: string, data: string): string { - return `data:${mimeType};base64,${data}`; -} - function formatRoleLabel(index: number, role: string): string { return `[${index}] role: ${role}`; } -function formatQuotedContent(content: string): string { - return `"${content}"`; -} +// PromptMessage/SamplingMessage content unions in the SDK are wider than +// ContentBlock (they admit tool_use, tool_result, etc. for the agent +// messages flowing into prompts). ContentViewer renders only the visual +// subset; everything else is silently dropped here. The bubble's role +// header keeps an empty message from being invisible. +const RENDERABLE_TYPES = new Set([ + "text", + "image", + "audio", + "resource", + "resource_link", +]); -interface ContentBlockRendered { - text: string; - imageUri?: string; - audioUri?: string; - audioMime?: string; +function isRenderableBlock(block: unknown): block is ContentBlock { + if (typeof block !== "object" || block === null) return false; + const t = (block as { type?: string }).type; + return typeof t === "string" && RENDERABLE_TYPES.has(t); } -function extractContent( - message: SamplingMessage | PromptMessage, -): ContentBlockRendered { - const content = message.content; - const blocks = Array.isArray(content) ? content : [content]; - let text = ""; - let imageUri: string | undefined; - let audioUri: string | undefined; - let audioMime: string | undefined; - - for (const block of blocks) { - switch (block.type) { - case "text": - text += block.text; - break; - case "image": - imageUri = buildDataUri(block.mimeType, block.data); - break; - case "audio": - audioUri = buildDataUri(block.mimeType, block.data); - audioMime = block.mimeType; - break; - case "resource": - text += - "text" in block.resource - ? block.resource.text - : `[resource: ${block.resource.uri}]`; - break; - case "resource_link": - text += `[resource: ${block.uri}]`; - break; - default: - text += `[${block.type}]`; - break; - } - } - - return { text, imageUri, audioUri, audioMime }; +// Prompt content blocks don't carry a mimeType on the text variant +// (SDK `TextContent` is just `{ type: "text", text }`). Render text as +// markdown by default so prompt prose with code fences, lists, and links +// looks like prose rather than a preformatted dump. Image / audio blocks +// already carry mimeType; ContentViewer routes them itself. +function effectiveMimeForBlock(block: ContentBlock): string | undefined { + if (block.type === "text") return "text/markdown"; + return undefined; } const BubbleContainer = Paper.withProps({ @@ -81,29 +56,29 @@ const RoleLabel = Text.withProps({ ff: "monospace", }); -const PreviewImage = Image.withProps({ - maw: 300, - radius: "sm", - mt: "xs", +const HeaderRow = Group.withProps({ + justify: "space-between", }); export function MessageBubble({ index, message }: MessageBubbleProps) { - const { text, imageUri, audioUri, audioMime } = extractContent(message); + const content = message.content; + const rawBlocks = Array.isArray(content) ? content : [content]; + const blocks = rawBlocks.filter(isRenderableBlock); return ( - + {formatRoleLabel(index, message.role)} - {text && } - - {text && {formatQuotedContent(text)}} - {imageUri && } - {audioUri && ( - - )} + + {blocks.map((block, blockIndex) => ( + + ))} ); diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx index af8fec99c..571ab9d95 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx @@ -1,9 +1,37 @@ +import { useState } from "react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; import { renderWithMantine, screen } from "../../../test/renderWithMantine"; import { PromptArgumentsForm } from "./PromptArgumentsForm"; +/** + * Wrapper that owns `argumentValues` state so completion tests can + * type multi-character input naturally — the production parent + * (PromptsScreen) is what holds this state, and the form is controlled + * via its onArgumentChange callback. + */ +function StatefulForm( + props: Omit< + React.ComponentProps, + "argumentValues" | "onArgumentChange" + > & { initialValues?: Record }, +) { + const { initialValues, ...rest } = props; + const [values, setValues] = useState>( + initialValues ?? {}, + ); + return ( + + setValues((prev) => ({ ...prev, [name]: value })) + } + /> + ); +} + const promptNoArgs: Prompt = { name: "summarize", description: "Summarize the given text into key points", @@ -143,4 +171,81 @@ describe("PromptArgumentsForm", () => { expect(screen.getByText("code-review")).toBeInTheDocument(); expect(screen.getByText("Arguments")).toBeInTheDocument(); }); + + describe("completions", () => { + it("calls onCompleteArgument (debounced) and surfaces values when supported", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi + .fn< + ( + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue(["alpha", "alphabet"]); + + renderWithMantine( + , + ); + + await user.type(screen.getByRole("textbox", { name: /^text/ }), "al"); + await new Promise((r) => setTimeout(r, 400)); + expect(onCompleteArgument).toHaveBeenCalled(); + expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "al", {}); + expect(await screen.findByText("alpha")).toBeInTheDocument(); + expect(screen.getByText("alphabet")).toBeInTheDocument(); + }); + + it("passes sibling argument values as completion context", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi + .fn< + ( + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue([]); + + renderWithMantine( + , + ); + + await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); + await new Promise((r) => setTimeout(r, 400)); + // The completing arg ("text") is excluded from context; siblings ride along. + expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "h", { + targetLanguage: "es", + }); + }); + + it("does not call onCompleteArgument when completions are unsupported", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi.fn(); + renderWithMantine( + , + ); + await user.type(screen.getByPlaceholderText("Enter text..."), "ab"); + await new Promise((r) => setTimeout(r, 400)); + expect(onCompleteArgument).not.toHaveBeenCalled(); + }); + }); }); diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx index bd3e72522..97eb16f42 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx @@ -1,4 +1,13 @@ -import { Button, Group, Stack, Text, TextInput, Title } from "@mantine/core"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { + Autocomplete, + Button, + Group, + Stack, + Text, + TextInput, + Title, +} from "@mantine/core"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; export interface PromptArgumentsFormProps { @@ -6,8 +15,27 @@ export interface PromptArgumentsFormProps { argumentValues: Record; onArgumentChange: (name: string, value: string) => void; onGetPrompt: () => void; + /** + * When provided, each keystroke in an argument input dispatches a + * (debounced) `completion/complete` request to the server and surfaces + * the returned values as a dropdown via Mantine `Autocomplete`. + * Wire to `InspectorClient.getCompletions` in the host App. + */ + onCompleteArgument?: ( + argumentName: string, + argumentValue: string, + context: Record, + ) => Promise; + /** + * Gates whether to render Autocomplete (with live completions) vs the + * plain TextInput. Typically derived from the server's + * `completions` capability. + */ + completionsSupported?: boolean; } +const COMPLETION_DEBOUNCE_MS = 300; + const PromptTitle = Text.withProps({ fw: 700, size: "lg", @@ -28,9 +56,82 @@ export function PromptArgumentsForm({ argumentValues, onArgumentChange, onGetPrompt, + onCompleteArgument, + completionsSupported = false, }: PromptArgumentsFormProps) { const { name, title, description, arguments: promptArguments } = prompt; + const [completions, setCompletions] = useState>({}); + + // Reset completion state whenever the active prompt changes — completions + // are keyed by argument name, and the same name could mean different + // things across prompts. + useEffect(() => { + setCompletions({}); + }, [name]); + + // Per-arg in-flight controller (later keystroke aborts older request). + const requestsRef = useRef>(new Map()); + // Per-arg debounce timer so we don't spam the server on every key. + const timersRef = useRef>>( + new Map(), + ); + + useEffect(() => { + const timers = timersRef.current; + const requests = requestsRef.current; + return () => { + for (const t of timers.values()) clearTimeout(t); + timers.clear(); + for (const c of requests.values()) c.abort(); + requests.clear(); + }; + }, []); + + const useAutocomplete = completionsSupported && !!onCompleteArgument; + + const runCompletion = useCallback( + async (argName: string, value: string, context: Record) => { + if (!onCompleteArgument) return; + requestsRef.current.get(argName)?.abort(); + const controller = new AbortController(); + requestsRef.current.set(argName, controller); + try { + const values = await onCompleteArgument(argName, value, context); + if (controller.signal.aborted) return; + setCompletions((prev) => ({ ...prev, [argName]: values })); + } catch { + if (!controller.signal.aborted) { + setCompletions((prev) => ({ ...prev, [argName]: [] })); + } + } finally { + if (requestsRef.current.get(argName) === controller) { + requestsRef.current.delete(argName); + } + } + }, + [onCompleteArgument], + ); + + function handleChange(argName: string, value: string) { + onArgumentChange(argName, value); + if (!useAutocomplete) return; + // The completing arg is excluded from context so the server can + // disambiguate when one argument depends on another. + const context: Record = { + ...argumentValues, + [argName]: value, + }; + delete context[argName]; + const existing = timersRef.current.get(argName); + if (existing) clearTimeout(existing); + const timer = setTimeout(() => { + timersRef.current.delete(argName); + void runCompletion(argName, value, context); + }, COMPLETION_DEBOUNCE_MS); + timersRef.current.set(argName, timer); + } + return ( {title ?? name} @@ -39,19 +140,36 @@ export function PromptArgumentsForm({ <> Arguments - {promptArguments.map((arg) => ( - - onArgumentChange(arg.name, event.currentTarget.value) - } - /> - ))} + {promptArguments.map((arg) => + useAutocomplete ? ( + options} + onChange={(value) => handleChange(arg.name, value)} + /> + ) : ( + + handleChange(arg.name, event.currentTarget.value) + } + /> + ), + )} )} diff --git a/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx b/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx index 0c882edd3..c9570fa06 100644 --- a/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx +++ b/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx @@ -1,4 +1,4 @@ -import { Button, Group, Stack, Text, Title } from "@mantine/core"; +import { Button, Group, ScrollArea, Stack, Text, Title } from "@mantine/core"; import type { PromptMessage } from "@modelcontextprotocol/sdk/types.js"; import { MessageBubble } from "../../elements/MessageBubble/MessageBubble"; @@ -12,25 +12,59 @@ const CopyAllButton = Button.withProps({ size: "sm", }); +// Outer stack inside the PreviewCard: header stays pinned, the scroll +// region absorbs overflow. Mirrors ResourcePreviewPanel so prompts and +// resources share the same sized-to-content / cap-then-scroll behavior. +const PanelStack = Stack.withProps({ + gap: "md", + miw: 0, + mih: 0, +}); + +const HeaderRow = Group.withProps({ + justify: "space-between", + flex: "0 0 auto", +}); + +// `0 1 auto` lets the scroll region shrink (but not grow) when the card +// hits its mah. `mih: 0` is required for flex children to shrink below +// their content's intrinsic height. +const MessagesScroll = ScrollArea.withProps({ + flex: "0 1 auto", + miw: 0, + mih: 0, + type: "auto", + scrollbars: "y", + offsetScrollbars: true, +}); + +const MessagesStack = Stack.withProps({ + gap: "md", +}); + export function PromptMessagesDisplay({ messages, onCopyAll, }: PromptMessagesDisplayProps) { return ( - - + + Messages {onCopyAll && messages.length > 0 && ( Copy All )} - - {messages.length === 0 ? ( - No messages to display - ) : ( - messages.map((message, index) => ( - - )) - )} - + + + + {messages.length === 0 ? ( + No messages to display + ) : ( + messages.map((message, index) => ( + + )) + )} + + + ); } diff --git a/clients/web/src/components/groups/SamplingRequestPanel/SamplingRequestPanel.test.tsx b/clients/web/src/components/groups/SamplingRequestPanel/SamplingRequestPanel.test.tsx index 7fca61610..bbda1b9d5 100644 --- a/clients/web/src/components/groups/SamplingRequestPanel/SamplingRequestPanel.test.tsx +++ b/clients/web/src/components/groups/SamplingRequestPanel/SamplingRequestPanel.test.tsx @@ -69,7 +69,7 @@ describe("SamplingRequestPanel", () => { ).toBeInTheDocument(); expect(screen.getByText("Messages:")).toBeInTheDocument(); expect( - screen.getByText('"What is the capital of France?"'), + screen.getByText("What is the capital of France?"), ).toBeInTheDocument(); }); diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx index 46a464c68..f1868a1c0 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx @@ -4,13 +4,26 @@ import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; import { renderWithMantine, screen } from "../../../test/renderWithMantine"; import { PromptsScreen } from "./PromptsScreen"; -const prompts: Prompt[] = [ - { name: "summarize", description: "Summarize text" }, - { name: "translate", description: "Translate text" }, +const promptsWithArgs: Prompt[] = [ + { + name: "summarize", + description: "Summarize text", + arguments: [{ name: "topic", required: true }], + }, + { + name: "translate", + description: "Translate text", + arguments: [{ name: "text", required: true }], + }, +]; + +const noArgPrompts: Prompt[] = [ + { name: "ping", description: "No-arg ping" }, + { name: "pong", description: "Also no-arg" }, ]; const baseProps = { - prompts, + prompts: promptsWithArgs, listChanged: false, onRefreshList: vi.fn(), onGetPrompt: vi.fn(), @@ -24,7 +37,7 @@ describe("PromptsScreen", () => { ).toBeInTheDocument(); }); - it("shows prompt arguments when a prompt is selected", async () => { + it("shows the argument form when a prompt with arguments is selected", async () => { const user = userEvent.setup(); renderWithMantine(); await user.click(screen.getByText("summarize")); @@ -33,88 +46,189 @@ describe("PromptsScreen", () => { ).toBeInTheDocument(); }); - it("shows pending state", async () => { + it("auto-fetches when a no-argument prompt is selected", async () => { const user = userEvent.setup(); + const onGetPrompt = vi.fn(); renderWithMantine( - , + , ); - await user.click(screen.getByText("summarize")); - expect(screen.getByText("Loading prompt...")).toBeInTheDocument(); + await user.click(screen.getByText("ping")); + expect(onGetPrompt).toHaveBeenCalledWith("ping", {}); + // The form pane is not rendered for no-argument prompts. + expect( + screen.queryByRole("button", { name: "Get Prompt" }), + ).not.toBeInTheDocument(); + }); + + it("does not re-fire auto-fetch on subsequent renders", async () => { + const user = userEvent.setup(); + const onGetPrompt = vi.fn(); + const { rerender } = renderWithMantine( + , + ); + await user.click(screen.getByText("ping")); + expect(onGetPrompt).toHaveBeenCalledTimes(1); + // Parent re-renders with a fresh pending state — the fetch must not + // re-fire just because props changed. + rerender( + , + ); + expect(onGetPrompt).toHaveBeenCalledTimes(1); }); - it("shows error state", async () => { + it("hides the argument form once the user clicks Get Prompt", async () => { const user = userEvent.setup(); + const onGetPrompt = vi.fn(); renderWithMantine( , ); await user.click(screen.getByText("summarize")); - expect(screen.getByText("Prompt Error")).toBeInTheDocument(); - expect(screen.getByText("Bad prompt")).toBeInTheDocument(); + await user.type(screen.getByPlaceholderText("Enter topic..."), "math"); + await user.click(screen.getByRole("button", { name: "Get Prompt" })); + expect(onGetPrompt).toHaveBeenCalledWith("summarize", { topic: "math" }); + // After submit, the form is gone and the messages panel is shown. + expect( + screen.queryByRole("button", { name: "Get Prompt" }), + ).not.toBeInTheDocument(); + expect(screen.getByText("Messages")).toBeInTheDocument(); }); - it("renders fallback error when error message is missing", async () => { + it("shows pending state once the user has submitted", async () => { const user = userEvent.setup(); renderWithMantine( - , + , ); await user.click(screen.getByText("summarize")); - expect(screen.getByText("Failed to get prompt")).toBeInTheDocument(); + await user.type(screen.getByPlaceholderText("Enter topic..."), "x"); + await user.click(screen.getByRole("button", { name: "Get Prompt" })); + expect(screen.getByText("Loading prompt...")).toBeInTheDocument(); }); - it("shows messages when result is provided", async () => { + it("shows error state once the user has submitted", async () => { const user = userEvent.setup(); renderWithMantine( , ); await user.click(screen.getByText("summarize")); - expect(screen.getByText("Messages")).toBeInTheDocument(); + await user.type(screen.getByPlaceholderText("Enter topic..."), "x"); + await user.click(screen.getByRole("button", { name: "Get Prompt" })); + expect(screen.getByText("Prompt Error")).toBeInTheDocument(); + expect(screen.getByText("Bad prompt")).toBeInTheDocument(); }); - it("updates argument values and calls onGetPrompt", async () => { + it("falls back to a default error message when none is provided", async () => { const user = userEvent.setup(); - const onGetPrompt = vi.fn(); - const promptsWithArgs: Prompt[] = [ - { - name: "ask", - arguments: [{ name: "topic", required: true }], - }, - ]; renderWithMantine( , ); - await user.click(screen.getByText("ask")); - await user.type(screen.getByPlaceholderText("Enter topic..."), "math"); + await user.click(screen.getByText("summarize")); + await user.type(screen.getByPlaceholderText("Enter topic..."), "x"); await user.click(screen.getByRole("button", { name: "Get Prompt" })); - expect(onGetPrompt).toHaveBeenCalledWith("ask", { topic: "math" }); + expect(screen.getByText("Failed to get prompt")).toBeInTheDocument(); + }); + + it("ignores a stale getPromptState whose name does not match the selection", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + await user.click(screen.getByText("summarize")); + // Form for the freshly-selected prompt, not the stale "translate" result. + expect( + screen.getByRole("button", { name: "Get Prompt" }), + ).toBeInTheDocument(); + expect(screen.queryByText("Messages")).not.toBeInTheDocument(); }); it("resets argument values when switching prompts", async () => { const user = userEvent.setup(); - const promptsWithArgs: Prompt[] = [ + const arglessTwoStep: Prompt[] = [ { name: "alpha", arguments: [{ name: "x" }] }, { name: "beta", arguments: [{ name: "y" }] }, ]; renderWithMantine( - , + , ); await user.click(screen.getByText("alpha")); await user.type(screen.getByPlaceholderText("Enter x..."), "value"); await user.click(screen.getByText("beta")); expect(screen.getByPlaceholderText("Enter y...")).toHaveValue(""); }); + + it("threads onCompleteArgument with a ref/prompt envelope", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi + .fn< + ( + ref: + | { type: "ref/resource"; uri: string } + | { type: "ref/prompt"; name: string }, + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue([]); + renderWithMantine( + , + ); + await user.click(screen.getByText("summarize")); + await user.type(screen.getByRole("textbox", { name: /topic/ }), "ab"); + await new Promise((r) => setTimeout(r, 400)); + expect(onCompleteArgument).toHaveBeenCalled(); + expect(onCompleteArgument.mock.calls[0][0]).toEqual({ + type: "ref/prompt", + name: "summarize", + }); + }); }); diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index 3e1484ed8..2a7bb108d 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -1,14 +1,5 @@ import { useState } from "react"; -import { - Alert, - Card, - Flex, - Group, - Loader, - ScrollArea, - Stack, - Text, -} from "@mantine/core"; +import { Alert, Card, Flex, Loader, Stack, Text } from "@mantine/core"; import type { GetPromptResult, Prompt, @@ -21,15 +12,29 @@ export interface GetPromptState { status: "idle" | "pending" | "ok" | "error"; result?: GetPromptResult; error?: string; + /** + * Name of the prompt the in-flight / latest result is for. Used to + * route the result panel only to the matching sidebar selection. + */ + promptName?: string; } export interface PromptsScreenProps { prompts: Prompt[]; getPromptState?: GetPromptState; listChanged: boolean; + completionsSupported?: boolean; onRefreshList: () => void; onGetPrompt: (name: string, args: Record) => void; onCopyMessages?: () => void; + onCompleteArgument?: ( + ref: + | { type: "ref/resource"; uri: string } + | { type: "ref/prompt"; name: string }, + argumentName: string, + argumentValue: string, + context: Record, + ) => Promise; } const ScreenLayout = Flex.withProps({ @@ -50,24 +55,50 @@ const SidebarCard = Card.withProps({ }); const DetailCard = Card.withProps({ - flex: 1, withBorder: true, padding: "lg", }); +// Sized-to-content card with overflow handling. When the inner content +// fits, the card hugs it. When it doesn't, the inner ScrollArea inside +// PromptMessagesDisplay shrinks (flex 0 1 auto, mih 0) and scrolls. +const PreviewCard = Card.withProps({ + withBorder: true, + padding: "lg", + variant: "preview", +}); + +// Column wrapper that pins the card to the top of the available space +// and bounds its growth via the consumer-set `mah`. +const PreviewPane = Flex.withProps({ + flex: 1, + miw: 0, + direction: "column", + align: "stretch", +}); + const EmptyState = Text.withProps({ c: "dimmed", ta: "center", py: "xl", }); +const SCROLL_MAX_HEIGHT = + "calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)"; + +function hasArguments(prompt: Prompt): boolean { + return !!prompt.arguments && prompt.arguments.length > 0; +} + export function PromptsScreen({ prompts, getPromptState, listChanged, + completionsSupported, onRefreshList, onGetPrompt, onCopyMessages, + onCompleteArgument, }: PromptsScreenProps) { const [selectedPromptName, setSelectedPromptName] = useState< string | undefined @@ -75,10 +106,82 @@ export function PromptsScreen({ const [argumentValues, setArgumentValues] = useState>( {}, ); + // Track which prompt the user has already submitted in this session so + // the form pane disappears once the user clicks Get Prompt. Cleared on + // sidebar switch so the form re-appears for a freshly-selected prompt. + const [submittedFor, setSubmittedFor] = useState( + undefined, + ); + const selectedPrompt = selectedPromptName ? prompts.find((p) => p.name === selectedPromptName) : undefined; + function handleSelectPrompt(name: string) { + setArgumentValues({}); + setSelectedPromptName(name); + // Auto-fetch no-argument prompts the moment they're selected — the + // form pane would otherwise just render a bare Get Prompt button + // with nothing to fill in. Prompts with arguments wait for submit. + const target = prompts.find((p) => p.name === name); + if (target && !hasArguments(target)) { + setSubmittedFor(name); + onGetPrompt(name, {}); + } else { + setSubmittedFor(undefined); + } + } + + function handleSubmit() { + if (!selectedPrompt) return; + setSubmittedFor(selectedPrompt.name); + onGetPrompt(selectedPrompt.name, argumentValues); + } + + // The preview is "active" when we've submitted (or auto-fetched) the + // currently-selected prompt and the parent's result/pending/error state + // matches. Without the name check, a stale result from a previous + // prompt would leak into the new prompt's pane. + const previewActive = + !!selectedPrompt && + submittedFor === selectedPrompt.name && + (!getPromptState?.promptName || + getPromptState.promptName === selectedPrompt.name); + + function renderPreview() { + if (!previewActive || !getPromptState) return null; + if (getPromptState.status === "pending") { + return ( + + + + Loading prompt... + + + ); + } + if (getPromptState.status === "error") { + return ( + + + {getPromptState.error ?? "Failed to get prompt"} + + + ); + } + if (getPromptState.result) { + return ( + + + + ); + } + return null; + } + return ( @@ -88,64 +191,48 @@ export function PromptsScreen({ selectedName={selectedPromptName} listChanged={listChanged} onRefreshList={onRefreshList} - onSelectPrompt={(name) => { - setArgumentValues({}); - setSelectedPromptName(name); - }} + onSelectPrompt={handleSelectPrompt} /> - - - {selectedPrompt ? ( - <> - - - setArgumentValues((prev) => ({ ...prev, [name]: value })) - } - onGetPrompt={() => - onGetPrompt(selectedPrompt.name, argumentValues) - } - /> - - {getPromptState?.status === "pending" && ( - - - - Loading prompt... - - - )} - {getPromptState?.status === "error" && ( - - - {getPromptState.error ?? "Failed to get prompt"} - - - )} - {getPromptState?.result && ( - - - - )} - - ) : ( - - Select a prompt to view details - - )} - - + {previewActive ? ( + // Result branch — sized to content, capped at viewport. Mirrors + // the resource preview layout (see ResourcesScreen). + {renderPreview()} + ) : selectedPrompt && hasArguments(selectedPrompt) ? ( + // Argument-form branch — capped at 40% width so the form doesn't + // stretch across the viewport on wide displays. Disappears once + // the user clicks Get Prompt and previewActive flips on. + + + + setArgumentValues((prev) => ({ ...prev, [argName]: value })) + } + onGetPrompt={handleSubmit} + completionsSupported={completionsSupported} + onCompleteArgument={ + onCompleteArgument + ? (argName, value, context) => + onCompleteArgument( + { type: "ref/prompt", name: selectedPrompt.name }, + argName, + value, + context, + ) + : undefined + } + /> + + + ) : ( + + Select a prompt to view details + + )} ); } diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index ef13e1416..0f0575b31 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -393,9 +393,11 @@ export function InspectorView({ prompts={prompts} getPromptState={getPromptState} listChanged={false} + completionsSupported={completionsSupported} onRefreshList={onRefreshPrompts} onGetPrompt={onGetPrompt} onCopyMessages={onCopyPromptMessages} + onCompleteArgument={onCompleteArgument} /> From b1b541b9c1ebd9b80a949f5c8dc7c0245ce3e6e8 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 20 May 2026 14:22:29 -0400 Subject: [PATCH 2/9] Add close (X) buttons to ResourcePreviewPanel and PromptMessagesDisplay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A top-left CloseButton dismisses the preview panel. The host screen decides what to fall back to: - ResourcesScreen remembers the originating template URI when the user reads from a template form (originatingTemplateUri); closing the preview restores the template form. Plain-resource reads (sidebar selection) just empty the selection and return to the empty state. Picking a different template / resource from the sidebar clears the back-trail. - PromptsScreen flips submittedFor back to undefined when the closed prompt has arguments — the argument form re-renders with the user's values preserved so they can edit and re-submit. No-arg prompts have nothing to fall back to, so the selection is dropped and the empty state appears. The X button is hidden when the host omits onClose, so callers that don't want a dismiss control keep their existing rendering. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PromptMessagesDisplay.test.tsx | 17 +++++ .../PromptMessagesDisplay.tsx | 29 ++++++++- .../ResourcePreviewPanel.test.tsx | 17 +++++ .../ResourcePreviewPanel.tsx | 20 +++++- .../PromptsScreen/PromptsScreen.test.tsx | 49 ++++++++++++++ .../screens/PromptsScreen/PromptsScreen.tsx | 14 ++++ .../ResourcesScreen/ResourcesScreen.test.tsx | 64 +++++++++++++++++++ .../ResourcesScreen/ResourcesScreen.tsx | 24 ++++++- 8 files changed, 230 insertions(+), 4 deletions(-) diff --git a/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.test.tsx b/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.test.tsx index 3f5cd322b..839dcd884 100644 --- a/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.test.tsx +++ b/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.test.tsx @@ -46,4 +46,21 @@ describe("PromptMessagesDisplay", () => { await user.click(screen.getByRole("button", { name: "Copy All" })); expect(onCopyAll).toHaveBeenCalledTimes(1); }); + + it("renders a close button when onClose is provided and invokes it on click", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Close messages" })); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("does not render a close button when onClose is omitted", () => { + renderWithMantine(); + expect( + screen.queryByRole("button", { name: "Close messages" }), + ).not.toBeInTheDocument(); + }); }); diff --git a/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx b/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx index c9570fa06..cc652b8a9 100644 --- a/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx +++ b/clients/web/src/components/groups/PromptMessagesDisplay/PromptMessagesDisplay.tsx @@ -1,10 +1,24 @@ -import { Button, Group, ScrollArea, Stack, Text, Title } from "@mantine/core"; +import { + Button, + CloseButton, + Group, + ScrollArea, + Stack, + Text, + Title, +} from "@mantine/core"; import type { PromptMessage } from "@modelcontextprotocol/sdk/types.js"; import { MessageBubble } from "../../elements/MessageBubble/MessageBubble"; export interface PromptMessagesDisplayProps { messages: PromptMessage[]; onCopyAll?: () => void; + /** + * When provided, a top-left X button dismisses the panel. The host + * (`PromptsScreen`) decides what to show in its place — typically + * the prompt's argument form (if it has arguments) or the empty state. + */ + onClose?: () => void; } const CopyAllButton = Button.withProps({ @@ -26,6 +40,11 @@ const HeaderRow = Group.withProps({ flex: "0 0 auto", }); +const HeaderLeft = Group.withProps({ + gap: "xs", + wrap: "nowrap", +}); + // `0 1 auto` lets the scroll region shrink (but not grow) when the card // hits its mah. `mih: 0` is required for flex children to shrink below // their content's intrinsic height. @@ -45,11 +64,17 @@ const MessagesStack = Stack.withProps({ export function PromptMessagesDisplay({ messages, onCopyAll, + onClose, }: PromptMessagesDisplayProps) { return ( - Messages + + {onClose && ( + + )} + Messages + {onCopyAll && messages.length > 0 && ( Copy All )} diff --git a/clients/web/src/components/groups/ResourcePreviewPanel/ResourcePreviewPanel.test.tsx b/clients/web/src/components/groups/ResourcePreviewPanel/ResourcePreviewPanel.test.tsx index d6db9ae6c..82528b793 100644 --- a/clients/web/src/components/groups/ResourcePreviewPanel/ResourcePreviewPanel.test.tsx +++ b/clients/web/src/components/groups/ResourcePreviewPanel/ResourcePreviewPanel.test.tsx @@ -237,6 +237,23 @@ describe("ResourcePreviewPanel", () => { ).toBeInTheDocument(); }); + it("renders a close button when onClose is provided and invokes it on click", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Close preview" })); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("does not render a close button when onClose is omitted", () => { + renderWithMantine(); + expect( + screen.queryByRole("button", { name: "Close preview" }), + ).not.toBeInTheDocument(); + }); + it("does not render plain-text content as markdown even with markdown-looking text", () => { renderWithMantine( void; onSubscribe: () => void; onUnsubscribe: () => void; + /** + * When provided, a top-left X button dismisses the panel. The host + * (`ResourcesScreen`) decides what to show in its place — either the + * originating template form or the empty state. + */ + onClose?: () => void; } function toContentBlock( @@ -57,6 +64,11 @@ const HeaderRow = Group.withProps({ flex: "0 0 auto", }); +const HeaderLeft = Group.withProps({ + gap: "xs", + wrap: "nowrap", +}); + const UriGroup = Group.withProps({ gap: "xs", wrap: "nowrap", @@ -159,6 +171,7 @@ export function ResourcePreviewPanel({ onRefresh, onSubscribe, onUnsubscribe, + onClose, }: ResourcePreviewPanelProps) { const { uri, annotations } = resource; const mimeType = effectiveMime(contents[0]?.mimeType, resource); @@ -166,7 +179,12 @@ export function ResourcePreviewPanel({ return ( - Resource + + {onClose && ( + + )} + Resource + {uri} diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx index f1868a1c0..15b7d9979 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx @@ -201,6 +201,55 @@ describe("PromptsScreen", () => { expect(screen.getByPlaceholderText("Enter y...")).toHaveValue(""); }); + it("closing the preview for an arg-bearing prompt brings the form back", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + await user.click(screen.getByText("summarize")); + await user.type(screen.getByPlaceholderText("Enter topic..."), "math"); + await user.click(screen.getByRole("button", { name: "Get Prompt" })); + // Preview is showing now — close it. + await user.click(screen.getByRole("button", { name: "Close messages" })); + expect( + screen.getByRole("button", { name: "Get Prompt" }), + ).toBeInTheDocument(); + // Argument value is preserved so the user can edit + re-submit. + expect(screen.getByPlaceholderText("Enter topic...")).toHaveValue("math"); + }); + + it("closing the preview for a no-arg prompt drops the selection", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + await user.click(screen.getByText("ping")); + await user.click(screen.getByRole("button", { name: "Close messages" })); + // No form to fall back to → empty state. + expect( + screen.getByText("Select a prompt to view details"), + ).toBeInTheDocument(); + }); + it("threads onCompleteArgument with a ref/prompt envelope", async () => { const user = userEvent.setup(); const onCompleteArgument = vi diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index 2a7bb108d..43934d88f 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -138,6 +138,19 @@ export function PromptsScreen({ onGetPrompt(selectedPrompt.name, argumentValues); } + function handleClosePreview() { + // For prompts with arguments, flip back to the form so the user can + // edit and re-submit (argumentValues are preserved). For no-arg + // prompts there's no form to return to, so drop the selection and + // fall back to the empty state. + if (selectedPrompt && hasArguments(selectedPrompt)) { + setSubmittedFor(undefined); + } else { + setSelectedPromptName(undefined); + setSubmittedFor(undefined); + } + } + // The preview is "active" when we've submitted (or auto-fetched) the // currently-selected prompt and the parent's result/pending/error state // matches. Without the name check, a stale result from a previous @@ -175,6 +188,7 @@ export function PromptsScreen({ ); diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx index 8991754a8..5574a86ba 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx @@ -165,6 +165,70 @@ describe("ResourcesScreen", () => { expect(onSubscribeResource).toHaveBeenCalledWith("file:///x"); }); + it("closing the preview returns to the originating template form", async () => { + const user = userEvent.setup(); + const onReadResource = vi.fn(); + const templates: ResourceTemplate[] = [ + { uriTemplate: "file:///{path}", name: "files" }, + ]; + const { rerender } = renderWithMantine( + , + ); + // Open the template form. + await user.click(screen.getByText("Templates (1)")); + await user.click(screen.getByText("files")); + // Submit it — the screen calls onReadResource and remembers the + // template URI for the close handler. + await user.type(screen.getByLabelText("path"), "alpha"); + await user.click(screen.getByRole("button", { name: "Read Resource" })); + expect(onReadResource).toHaveBeenCalledWith("file:///alpha"); + + // Parent re-renders with the read result; the preview appears. + rerender( + , + ); + expect( + screen.queryByRole("button", { name: "Read Resource" }), + ).not.toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Close preview" })); + // Closing brings the template form back. + expect( + screen.getByRole("button", { name: "Read Resource" }), + ).toBeInTheDocument(); + }); + + it("closing the preview for a plain resource returns to the empty state", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + await user.click(screen.getByText("x.txt")); + await user.click(screen.getByRole("button", { name: "Close preview" })); + expect( + screen.getByText("Select a resource to preview"), + ).toBeInTheDocument(); + }); + it("invokes onUnsubscribeResource when already subscribed", async () => { const user = userEvent.setup(); const onUnsubscribeResource = vi.fn(); diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx index b22806e83..10f55f149 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx @@ -110,6 +110,13 @@ export function ResourcesScreen({ const [selectedTemplateUri, setSelectedTemplateUri] = useState< string | undefined >(undefined); + // Tracks which template (if any) produced the current preview so that + // closing the preview can restore the template form. Cleared when the + // user navigates to a non-template resource or picks a different + // template directly from the sidebar. + const [originatingTemplateUri, setOriginatingTemplateUri] = useState< + string | undefined + >(undefined); const selectedResource = selectedResourceUri ? resources.find((r) => r.uri === selectedResourceUri) @@ -129,24 +136,38 @@ export function ResourcesScreen({ function handleSelectResource(uri: string) { setSelectedTemplateUri(undefined); setSelectedResourceUri(uri); + setOriginatingTemplateUri(undefined); onReadResource(uri); } function handleSelectTemplate(uriTemplate: string) { setSelectedResourceUri(undefined); setSelectedTemplateUri(uriTemplate); + setOriginatingTemplateUri(undefined); } function handleReadResource(uri: string) { // Once the user reads (either from the template form or a refresh // inside the preview panel), hand the screen over to the preview: // clearing the template selection hides the template form so only - // the rendered resource is shown. + // the rendered resource is shown. We remember the template URI so + // closing the preview can restore the form. + if (selectedTemplateUri) { + setOriginatingTemplateUri(selectedTemplateUri); + } setSelectedTemplateUri(undefined); setSelectedResourceUri(uri); onReadResource(uri); } + function handleClosePreview() { + setSelectedResourceUri(undefined); + if (originatingTemplateUri) { + setSelectedTemplateUri(originatingTemplateUri); + setOriginatingTemplateUri(undefined); + } + } + function renderReadState() { if (!readState) return null; @@ -182,6 +203,7 @@ export function ResourcesScreen({ onRefresh={() => handleReadResource(readResource.uri)} onSubscribe={() => onSubscribeResource(readResource.uri)} onUnsubscribe={() => onUnsubscribeResource(readResource.uri)} + onClose={handleClosePreview} /> ); From 6d758749847d883c0207de6dc481d76c8f658d4c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 20 May 2026 14:37:53 -0400 Subject: [PATCH 3/9] Add close button to pending and error preview states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OK-state preview already had its X button (inside the panel component itself). Pending and error states were inline cards in renderReadState / renderPreview with no way to dismiss them, so a user who submitted bad input to a resource template or prompt was stuck staring at the error until they picked a different sidebar item. Adds a top-left CloseButton row to both states on both screens, wired to the same handleClosePreview handler — so the template form or argument form re-appears on close just like it does from the OK state. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PromptsScreen/PromptsScreen.test.tsx | 22 +++++++++++ .../screens/PromptsScreen/PromptsScreen.tsx | 39 +++++++++++++++---- .../ResourcesScreen/ResourcesScreen.test.tsx | 33 ++++++++++++++++ .../ResourcesScreen/ResourcesScreen.tsx | 39 +++++++++++++++---- 4 files changed, 119 insertions(+), 14 deletions(-) diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx index 15b7d9979..482fed2a7 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx @@ -201,6 +201,28 @@ describe("PromptsScreen", () => { expect(screen.getByPlaceholderText("Enter y...")).toHaveValue(""); }); + it("closing the preview from the error state brings the form back", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + await user.click(screen.getByText("summarize")); + await user.type(screen.getByPlaceholderText("Enter topic..."), "x"); + await user.click(screen.getByRole("button", { name: "Get Prompt" })); + expect(screen.getByText("Prompt Error")).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Close messages" })); + expect( + screen.getByRole("button", { name: "Get Prompt" }), + ).toBeInTheDocument(); + }); + it("closing the preview for an arg-bearing prompt brings the form back", async () => { const user = userEvent.setup(); renderWithMantine( diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index 43934d88f..6b82373b9 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -1,5 +1,14 @@ import { useState } from "react"; -import { Alert, Card, Flex, Loader, Stack, Text } from "@mantine/core"; +import { + Alert, + Card, + CloseButton, + Flex, + Group, + Loader, + Stack, + Text, +} from "@mantine/core"; import type { GetPromptResult, Prompt, @@ -166,9 +175,17 @@ export function PromptsScreen({ if (getPromptState.status === "pending") { return ( - - - Loading prompt... + + + + + + + Loading prompt... + ); @@ -176,9 +193,17 @@ export function PromptsScreen({ if (getPromptState.status === "error") { return ( - - {getPromptState.error ?? "Failed to get prompt"} - + + + + + + {getPromptState.error ?? "Failed to get prompt"} + + ); } diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx index 5574a86ba..efbca1bf4 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx @@ -210,6 +210,39 @@ describe("ResourcesScreen", () => { ).toBeInTheDocument(); }); + it("closing the preview from the error state returns to the template form", async () => { + const user = userEvent.setup(); + const templates: ResourceTemplate[] = [ + { uriTemplate: "demo://resource/dynamic/text/{id}", name: "Dynamic" }, + ]; + const { rerender } = renderWithMantine( + , + ); + await user.click(screen.getByText("Templates (1)")); + await user.click(screen.getByText("Dynamic")); + await user.type(screen.getByLabelText("id"), "asdf"); + await user.click(screen.getByRole("button", { name: "Read Resource" })); + + // Server rejects the URI. + rerender( + , + ); + expect(screen.getByText("Read Error")).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Close preview" })); + // The template form is restored so the user can fix their input. + expect( + screen.getByRole("button", { name: "Read Resource" }), + ).toBeInTheDocument(); + }); + it("closing the preview for a plain resource returns to the empty state", async () => { const user = userEvent.setup(); renderWithMantine( diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx index 10f55f149..bb8206a0e 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx @@ -1,5 +1,14 @@ import { useState } from "react"; -import { Alert, Card, Flex, Loader, Stack, Text } from "@mantine/core"; +import { + Alert, + Card, + CloseButton, + Flex, + Group, + Loader, + Stack, + Text, +} from "@mantine/core"; import type { ReadResourceResult, Resource, @@ -174,9 +183,17 @@ export function ResourcesScreen({ if (readState.status === "pending") { return ( - - - Reading resource... + + + + + + + Reading resource... + ); @@ -185,9 +202,17 @@ export function ResourcesScreen({ if (readState.status === "error") { return ( - - {readState.error ?? "Failed to read resource"} - + + + + + + {readState.error ?? "Failed to read resource"} + + ); } From 5408e164d5ccc434809b60ffd8a8564a7b46882c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 20 May 2026 14:53:57 -0400 Subject: [PATCH 4/9] Fire prompt completions on focus and send every sibling in context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the PromptArgumentsForm autocomplete: - Focusing an argument input now fires completion/complete immediately (handleFocus) — the dropdown is populated as soon as the user clicks in, not only after they start typing. Any in-flight debounce timer for the same arg is cancelled so a stale keystroke request can't overwrite the fresh focus response. - The completion context now includes every declared prompt argument (with "" for ones the user hasn't typed into yet), minus the one being completed. Previously the context only carried args the user had touched, so servers that disambiguate based on co-arguments couldn't see the full picture on the first keystroke. Tests cover both: a focus-only path that fires before any keystroke, and a typing path that asserts the empty sibling is sent through. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PromptArgumentsForm.test.tsx | 45 +++++++++++++++++-- .../PromptArgumentsForm.tsx | 38 +++++++++++++--- 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx index 571ab9d95..7338f1f87 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx @@ -173,6 +173,37 @@ describe("PromptArgumentsForm", () => { }); describe("completions", () => { + it("fires a completion immediately on focus before any keystroke", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi + .fn< + ( + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue(["alpha", "alphabet"]); + + renderWithMantine( + , + ); + + await user.click(screen.getByRole("textbox", { name: /^text/ })); + // No debounce on focus — the call fires synchronously off the + // focus handler. A microtask is enough for the response to settle. + await new Promise((r) => setTimeout(r, 0)); + expect(onCompleteArgument).toHaveBeenCalledWith("text", "", { + targetLanguage: "", + }); + expect(await screen.findByText("alpha")).toBeInTheDocument(); + }); + it("calls onCompleteArgument (debounced) and surfaces values when supported", async () => { const user = userEvent.setup(); const onCompleteArgument = vi @@ -196,13 +227,16 @@ describe("PromptArgumentsForm", () => { await user.type(screen.getByRole("textbox", { name: /^text/ }), "al"); await new Promise((r) => setTimeout(r, 400)); - expect(onCompleteArgument).toHaveBeenCalled(); - expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "al", {}); + // The last call carries the typed value and the context for the + // other (still empty) sibling. + expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "al", { + targetLanguage: "", + }); expect(await screen.findByText("alpha")).toBeInTheDocument(); expect(screen.getByText("alphabet")).toBeInTheDocument(); }); - it("passes sibling argument values as completion context", async () => { + it("sends every sibling argument in context, including the unset ones", async () => { const user = userEvent.setup(); const onCompleteArgument = vi .fn< @@ -226,7 +260,8 @@ describe("PromptArgumentsForm", () => { await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); await new Promise((r) => setTimeout(r, 400)); - // The completing arg ("text") is excluded from context; siblings ride along. + // The completing arg ("text") is excluded from context; every + // other declared argument rides along — even ones still empty. expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "h", { targetLanguage: "es", }); @@ -243,6 +278,8 @@ describe("PromptArgumentsForm", () => { onCompleteArgument={onCompleteArgument} />, ); + // Focus the input first, then type — neither should trigger a call. + await user.click(screen.getByPlaceholderText("Enter text...")); await user.type(screen.getByPlaceholderText("Enter text..."), "ab"); await new Promise((r) => setTimeout(r, 400)); expect(onCompleteArgument).not.toHaveBeenCalled(); diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx index 97eb16f42..0c16d9868 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx @@ -113,16 +113,25 @@ export function PromptArgumentsForm({ [onCompleteArgument], ); + // Build the `context.arguments` payload for a completion request. + // Includes every prompt argument the user could fill in (with `""` + // for ones they haven't typed yet) except the one being completed — + // the completing arg goes in `params.argument`. Servers that + // disambiguate based on co-arguments need all of them, not just + // whatever the user has already typed. + function buildContext(currentArg: string): Record { + const ctx: Record = {}; + for (const a of promptArguments ?? []) { + if (a.name === currentArg) continue; + ctx[a.name] = argumentValues[a.name] ?? ""; + } + return ctx; + } + function handleChange(argName: string, value: string) { onArgumentChange(argName, value); if (!useAutocomplete) return; - // The completing arg is excluded from context so the server can - // disambiguate when one argument depends on another. - const context: Record = { - ...argumentValues, - [argName]: value, - }; - delete context[argName]; + const context = buildContext(argName); const existing = timersRef.current.get(argName); if (existing) clearTimeout(existing); const timer = setTimeout(() => { @@ -132,6 +141,20 @@ export function PromptArgumentsForm({ timersRef.current.set(argName, timer); } + function handleFocus(argName: string) { + if (!useAutocomplete) return; + // Fire immediately so the dropdown isn't empty when the user first + // clicks in. Cancel any pending debounce so a stale keystroke + // request doesn't overwrite this fresher one. + const existing = timersRef.current.get(argName); + if (existing) { + clearTimeout(existing); + timersRef.current.delete(argName); + } + const value = argumentValues[argName] ?? ""; + void runCompletion(argName, value, buildContext(argName)); + } + return ( {title ?? name} @@ -155,6 +178,7 @@ export function PromptArgumentsForm({ // suggestions client-side. filter={({ options }) => options} onChange={(value) => handleChange(arg.name, value)} + onFocus={() => handleFocus(arg.name)} /> ) : ( Date: Wed, 20 May 2026 15:03:15 -0400 Subject: [PATCH 5/9] Fire resource-template completions on focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the prompt-side change: focusing a template variable input now fires completion/complete immediately so the dropdown populates the moment the user clicks in, rather than waiting for the first keystroke + 300ms debounce. Any pending debounce timer for the same variable is cancelled first so a stale keystroke response can't overwrite the focus response. The sibling-context coverage was already correct here — `variables` is seeded with empty strings for every declared template variable at mount and on template switch, so the context payload always carries the full variable set minus the one being completed. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ResourceTemplatePanel.test.tsx | 37 ++++++++++++++++++- .../ResourceTemplatePanel.tsx | 19 ++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx index c9a7ef842..7005c5bfe 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx @@ -139,6 +139,37 @@ describe("ResourceTemplatePanel", () => { }); describe("completions", () => { + it("fires a completion immediately on focus before any keystroke", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi + .fn< + ( + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue(["alpha", "alphabet"]); + + renderWithMantine( + , + ); + + await user.click(screen.getByRole("textbox", { name: "tableName" })); + await new Promise((r) => setTimeout(r, 0)); + // Empty value, empty sibling — but the sibling key is still + // present so the server sees the full argument set. + expect(onCompleteArgument).toHaveBeenCalledWith("tableName", "", { + rowId: "", + }); + expect(await screen.findByText("alpha")).toBeInTheDocument(); + }); + it("calls onCompleteArgument (debounced) and surfaces values when supported", async () => { const user = userEvent.setup(); const onCompleteArgument = vi @@ -163,8 +194,10 @@ describe("ResourceTemplatePanel", () => { await user.type(screen.getByRole("textbox", { name: "userId" }), "al"); // Wait past the 300ms debounce. await new Promise((r) => setTimeout(r, 400)); - expect(onCompleteArgument).toHaveBeenCalledTimes(1); - expect(onCompleteArgument).toHaveBeenCalledWith("userId", "al", {}); + // user.type focuses first (firing one immediate completion) and + // then types the characters (firing the debounced one). Only the + // typed-prefix call is the one we care about here. + expect(onCompleteArgument).toHaveBeenLastCalledWith("userId", "al", {}); // Server-returned values surface in the Autocomplete dropdown. expect(await screen.findByText("alpha")).toBeInTheDocument(); diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx index 1368fa9bc..1b857d2f6 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx @@ -184,6 +184,24 @@ export function ResourceTemplatePanel({ }); } + function handleVariableFocus(varName: string) { + if (!useAutocomplete) return; + // Fire immediately so the dropdown isn't empty when the user first + // clicks in. Cancel any pending debounce for this variable so a + // stale keystroke request doesn't overwrite the fresher focus + // response. `variables` already carries every declared template + // variable (seeded with "") so the context is complete by default. + const existing = timersRef.current.get(varName); + if (existing) { + clearTimeout(existing); + timersRef.current.delete(varName); + } + const value = variables[varName] ?? ""; + const context: Record = { ...variables }; + delete context[varName]; + void runCompletion(varName, value, context); + } + const canSubmit = variableNames.every((n) => variables[n]?.length > 0); function handleSubmit() { @@ -217,6 +235,7 @@ export function ResourceTemplatePanel({ // substring-match what the server returned. filter={({ options }) => options} onChange={(value) => handleVariableChange(varName, value)} + onFocus={() => handleVariableFocus(varName)} /> ) : ( Date: Wed, 20 May 2026 17:23:50 -0400 Subject: [PATCH 6/9] Style Autocomplete dropdowns to match the rest of the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes so the completion dropdowns on prompt-argument and resource-template inputs look like every other dropdown in the app: - App.css: the dark-mode dropdown / option-hover rules used to target .mantine-Select-* only, so the Autocomplete dropdowns shipped with Mantine's default surface color and a different hover background. Re-scoped to .mantine-Combobox-* so every Combobox-built input (Select, Autocomplete, MultiSelect, …) inherits the same styling. - theme/Autocomplete.ts: new theme module setting `radius: "md"` so Autocomplete matches the default Select / TextInput corner radius. Wired through theme/index.ts and theme/theme.ts. Co-Authored-By: Claude Opus 4.7 (1M context) --- clients/web/src/App.css | 15 +++++++++++---- clients/web/src/theme/Autocomplete.ts | 7 +++++++ clients/web/src/theme/index.ts | 1 + clients/web/src/theme/theme.ts | 2 ++ 4 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 clients/web/src/theme/Autocomplete.ts diff --git a/clients/web/src/App.css b/clients/web/src/App.css index 703644ef3..b2aa5715e 100644 --- a/clients/web/src/App.css +++ b/clients/web/src/App.css @@ -110,13 +110,20 @@ background-color: var(--mantine-primary-color-light); } -/* ── Select dropdown / option (dark mode) ──────────────────────── */ - -[data-mantine-color-scheme="dark"] .mantine-Select-dropdown { +/* ── Combobox dropdown / option (dark mode) ────────────────────── + * + * Targets `.mantine-Combobox-*` so the same dark-mode dropdown + * background + hover apply to every Combobox-built input (Select, + * Autocomplete, MultiSelect, TagsInput, …). Earlier this scoped to + * `.mantine-Select-*` only and the argument-completion Autocompletes + * shipped an unstyled dropdown that looked nothing like the rest of + * the app's filter dropdowns. */ + +[data-mantine-color-scheme="dark"] .mantine-Combobox-dropdown { background-color: var(--mantine-color-gray-8); } -[data-mantine-color-scheme="dark"] .mantine-Select-option:hover { +[data-mantine-color-scheme="dark"] .mantine-Combobox-option:hover { background-color: var(--mantine-primary-color-light); } diff --git a/clients/web/src/theme/Autocomplete.ts b/clients/web/src/theme/Autocomplete.ts new file mode 100644 index 000000000..e2ab27503 --- /dev/null +++ b/clients/web/src/theme/Autocomplete.ts @@ -0,0 +1,7 @@ +import { Autocomplete } from "@mantine/core"; + +export const ThemeAutocomplete = Autocomplete.extend({ + defaultProps: { + radius: "md", + }, +}); diff --git a/clients/web/src/theme/index.ts b/clients/web/src/theme/index.ts index 42b0cd1e3..1c3ee13d3 100644 --- a/clients/web/src/theme/index.ts +++ b/clients/web/src/theme/index.ts @@ -1,6 +1,7 @@ export { ThemeActionIcon } from "./ActionIcon"; export { ThemeAlert } from "./Alert"; export { ThemeAppShell } from "./AppShell"; +export { ThemeAutocomplete } from "./Autocomplete"; export { ThemeBadge } from "./Badge"; export { ThemeButton } from "./Button"; export { ThemeCard } from "./Card"; diff --git a/clients/web/src/theme/theme.ts b/clients/web/src/theme/theme.ts index bacebf3c9..cdbfc7555 100644 --- a/clients/web/src/theme/theme.ts +++ b/clients/web/src/theme/theme.ts @@ -3,6 +3,7 @@ import { ThemeActionIcon, ThemeAlert, ThemeAppShell, + ThemeAutocomplete, ThemeBadge, ThemeButton, ThemeCard, @@ -55,6 +56,7 @@ export const theme = createTheme({ ActionIcon: ThemeActionIcon, Alert: ThemeAlert, AppShell: ThemeAppShell, + Autocomplete: ThemeAutocomplete, Badge: ThemeBadge, Button: ThemeButton, Card: ThemeCard, From 719e7bec1e7f456f6530c4c84a0e3aeb9c0165f4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 20 May 2026 18:12:09 -0400 Subject: [PATCH 7/9] =?UTF-8?q?Fix=20dropdown=20styling=20=E2=80=94=20Sele?= =?UTF-8?q?ct=20and=20Autocomplete=20need=20separate=20selectors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit re-scoped the dark-mode dropdown rules from `.mantine-Select-*` to `.mantine-Combobox-*` on the assumption that both Select and Autocomplete share a Combobox class on the dropdown root. They don't — Mantine's Combobox factory takes `__staticSelector` from the parent component, so Select emits `mantine-Select-dropdown` and Autocomplete emits `mantine-Autocomplete-dropdown` with no shared `Combobox` class on that element. The `.mantine-Combobox-*` selector matched nothing, so the TaskControls filter (and every other Select) lost its gray surface and reverted to Mantine's default brown-ish dark background. List both selectors so Select dropdowns keep their styling and Autocomplete dropdowns pick it up. Co-Authored-By: Claude Opus 4.7 (1M context) --- clients/web/src/App.css | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/clients/web/src/App.css b/clients/web/src/App.css index b2aa5715e..b73f7a192 100644 --- a/clients/web/src/App.css +++ b/clients/web/src/App.css @@ -110,20 +110,22 @@ background-color: var(--mantine-primary-color-light); } -/* ── Combobox dropdown / option (dark mode) ────────────────────── +/* ── Select / Autocomplete dropdown / option (dark mode) ───────── * - * Targets `.mantine-Combobox-*` so the same dark-mode dropdown - * background + hover apply to every Combobox-built input (Select, - * Autocomplete, MultiSelect, TagsInput, …). Earlier this scoped to - * `.mantine-Select-*` only and the argument-completion Autocompletes - * shipped an unstyled dropdown that looked nothing like the rest of - * the app's filter dropdowns. */ - -[data-mantine-color-scheme="dark"] .mantine-Combobox-dropdown { + * Select and Autocomplete each render with their own static selector + * (`mantine-Select-*` / `mantine-Autocomplete-*`) — they share the + * Combobox primitive but don't emit a shared `mantine-Combobox-*` + * class on the dropdown root. List both so the argument-completion + * Autocompletes get the same gray surface + primary-light hover as + * the filter dropdowns. */ + +[data-mantine-color-scheme="dark"] .mantine-Select-dropdown, +[data-mantine-color-scheme="dark"] .mantine-Autocomplete-dropdown { background-color: var(--mantine-color-gray-8); } -[data-mantine-color-scheme="dark"] .mantine-Combobox-option:hover { +[data-mantine-color-scheme="dark"] .mantine-Select-option:hover, +[data-mantine-color-scheme="dark"] .mantine-Autocomplete-option:hover { background-color: var(--mantine-primary-color-light); } From 622270d7b87cfe8f5ed81c88d1718ec27ef5a642 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 20 May 2026 19:08:06 -0400 Subject: [PATCH 8/9] Address review feedback: canSubmit, fire-time context, re-click guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PromptArgumentsForm: add a canSubmit guard that disables Get Prompt until every required argument has a value, matching the resource template form's symmetry. Optional args may stay blank. - Both forms: capture sibling context inside the debounce callback, not at schedule time. Hold the latest argumentValues / variables in a ref + sync via useEffect, and call buildContext() at fire time. Without this, typing in arg A then arg B within the 300ms window would ship A's request with B's pre-keystroke value. - PromptsScreen.handleSelectPrompt: early-return when the user re-clicks the already-selected prompt — sidebar is for navigation, ✕ is for dismiss. Previously a re-click wiped form values and re-fired the auto-fetch for no-arg prompts. - MessageBubble: expand the comment on effectiveMimeForBlock to flag the unconditional markdown promotion as a known asymmetry with the resource side (which only promotes when the server signals it), pending a per-block mimeType in the spec. - New tests: canSubmit disabled / enabled transitions, fire-time context capture across siblings, abort-path verification (a stale in-flight response must not overwrite the fresh one), and the no-op re-click guard on PromptsScreen. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../elements/MessageBubble/MessageBubble.tsx | 9 ++ .../PromptArgumentsForm.test.tsx | 116 +++++++++++++++++- .../PromptArgumentsForm.tsx | 44 +++++-- .../ResourceTemplatePanel.tsx | 50 ++++---- .../PromptsScreen/PromptsScreen.test.tsx | 18 +++ .../screens/PromptsScreen/PromptsScreen.tsx | 5 + 6 files changed, 207 insertions(+), 35 deletions(-) diff --git a/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx b/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx index 29647aa1d..2c43c37d4 100644 --- a/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx +++ b/clients/web/src/components/elements/MessageBubble/MessageBubble.tsx @@ -39,6 +39,15 @@ function isRenderableBlock(block: unknown): block is ContentBlock { // markdown by default so prompt prose with code fences, lists, and links // looks like prose rather than a preformatted dump. Image / audio blocks // already carry mimeType; ContentViewer routes them itself. +// +// Caveat: this is unconditional — a server that emits a raw shell +// snippet, log line, or string containing `#` / `_` / backticks will +// have it transformed. Most prompts are prose so the trade-off is +// worth it, but this differs from the resource side (where +// ResourcePreviewPanel only promotes to markdown when the server +// supplies `text/markdown` or the URI suffix matches). If the MCP +// spec ever adds a per-block mimeType for prompt messages, switch +// back to opt-in rendering here. function effectiveMimeForBlock(block: ContentBlock): string | undefined { if (block.type === "text") return "text/markdown"; return undefined; diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx index 7338f1f87..99f55ddc6 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx @@ -150,7 +150,8 @@ describe("PromptArgumentsForm", () => { renderWithMantine( , @@ -159,6 +160,33 @@ describe("PromptArgumentsForm", () => { expect(onGetPrompt).toHaveBeenCalledTimes(1); }); + it("disables Get Prompt until every required argument has a value", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + const button = screen.getByRole("button", { name: "Get Prompt" }); + expect(button).toBeDisabled(); + await user.type(screen.getByPlaceholderText("Enter text..."), "hi"); + expect(button).not.toBeDisabled(); + }); + + it("allows submission when only optional arguments are blank", () => { + renderWithMantine( + , + ); + // targetLanguage is required: false, so leaving it blank should + // not disable submission. + expect( + screen.getByRole("button", { name: "Get Prompt" }), + ).not.toBeDisabled(); + }); + it("renders without description when none is provided", () => { renderWithMantine( { }); }); + it("captures sibling values at fire time, not at schedule time", async () => { + const user = userEvent.setup(); + const onCompleteArgument = vi + .fn< + ( + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue([]); + + renderWithMantine( + , + ); + + // Type into "text" — this schedules a debounced completion call. + await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); + // Before the 300ms debounce fires, update the sibling. The + // text-arg fire that lands at t=300 must see the latest sibling + // value, not the empty one captured at schedule time. + await user.type( + screen.getByRole("textbox", { name: /targetLanguage/ }), + "es", + ); + await new Promise((r) => setTimeout(r, 400)); + + // The most recent call for "text" carries the up-to-date + // sibling value, even though it was scheduled before "es" was + // typed. (There's also a focus-fire call when the second input + // gained focus — separate stream, not asserted here.) + const textCalls = onCompleteArgument.mock.calls.filter( + ([n]) => n === "text", + ); + expect(textCalls.at(-1)).toEqual(["text", "h", { targetLanguage: "es" }]); + }); + + it("aborts an in-flight request when a faster keystroke arrives", async () => { + const user = userEvent.setup(); + const calls: Array<{ + value: string; + resolve: (values: string[]) => void; + }> = []; + const onCompleteArgument = vi.fn( + (_argName: string, value: string) => + new Promise((resolve) => { + calls.push({ value, resolve }); + }), + ); + + renderWithMantine( + , + ); + + // Focus fires the first call (value=""). Type "h" → second call + // after debounce. Type "i" → third call after debounce. + await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); + await new Promise((r) => setTimeout(r, 350)); + await user.type(screen.getByRole("textbox", { name: /^text/ }), "i"); + await new Promise((r) => setTimeout(r, 350)); + + // Resolve the late "h" response — it should be dropped because + // the form aborted that controller when the "hi" request started. + const hi = calls.find((c) => c.value === "hi"); + const h = calls.find((c) => c.value === "h"); + expect(hi).toBeDefined(); + expect(h).toBeDefined(); + h?.resolve(["from-stale-h"]); + hi?.resolve(["from-fresh-hi"]); + await new Promise((r) => setTimeout(r, 0)); + + // The dropdown shows the fresh response, not the stale one. + expect(await screen.findByText("from-fresh-hi")).toBeInTheDocument(); + expect(screen.queryByText("from-stale-h")).not.toBeInTheDocument(); + }); + it("does not call onCompleteArgument when completions are unsupported", async () => { const user = userEvent.setup(); const onCompleteArgument = vi.fn(); diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx index 0c16d9868..f93f51b33 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx @@ -113,30 +113,43 @@ export function PromptArgumentsForm({ [onCompleteArgument], ); + // Hold the latest argumentValues in a ref so debounced fires can read + // sibling values at *fire* time, not at schedule time. Without this, + // typing in arg A then arg B within the debounce window would ship + // A's request with B's value stuck at its pre-keystroke state. + const argumentValuesRef = useRef(argumentValues); + useEffect(() => { + argumentValuesRef.current = argumentValues; + }, [argumentValues]); + // Build the `context.arguments` payload for a completion request. // Includes every prompt argument the user could fill in (with `""` // for ones they haven't typed yet) except the one being completed — // the completing arg goes in `params.argument`. Servers that // disambiguate based on co-arguments need all of them, not just // whatever the user has already typed. - function buildContext(currentArg: string): Record { - const ctx: Record = {}; - for (const a of promptArguments ?? []) { - if (a.name === currentArg) continue; - ctx[a.name] = argumentValues[a.name] ?? ""; - } - return ctx; - } + const buildContext = useCallback( + (currentArg: string): Record => { + const ctx: Record = {}; + for (const a of promptArguments ?? []) { + if (a.name === currentArg) continue; + ctx[a.name] = argumentValuesRef.current[a.name] ?? ""; + } + return ctx; + }, + [promptArguments], + ); function handleChange(argName: string, value: string) { onArgumentChange(argName, value); if (!useAutocomplete) return; - const context = buildContext(argName); const existing = timersRef.current.get(argName); if (existing) clearTimeout(existing); const timer = setTimeout(() => { timersRef.current.delete(argName); - void runCompletion(argName, value, context); + // Build context at fire time so sibling values that arrived + // between schedule and fire are picked up. + void runCompletion(argName, value, buildContext(argName)); }, COMPLETION_DEBOUNCE_MS); timersRef.current.set(argName, timer); } @@ -151,10 +164,17 @@ export function PromptArgumentsForm({ clearTimeout(existing); timersRef.current.delete(argName); } - const value = argumentValues[argName] ?? ""; + const value = argumentValuesRef.current[argName] ?? ""; void runCompletion(argName, value, buildContext(argName)); } + // Mirror ResourceTemplatePanel: every required argument must be + // filled before Get Prompt is enabled. Optional args are allowed to + // stay empty; the server will treat them as absent. + const canSubmit = (promptArguments ?? []) + .filter((a) => a.required === true) + .every((a) => (argumentValues[a.name] ?? "").length > 0); + return ( {title ?? name} @@ -198,7 +218,7 @@ export function PromptArgumentsForm({ )} - diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx index 1b857d2f6..89e672d00 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx @@ -163,25 +163,33 @@ export function ResourceTemplatePanel({ [onCompleteArgument], ); + // Hold the latest `variables` in a ref so a debounced completion + // call reads sibling values at fire time, not at schedule time. + // Typing in A then B within the 300ms window would otherwise ship + // A's request with B's value still empty in context. + const variablesRef = useRef(variables); + useEffect(() => { + variablesRef.current = variables; + }, [variables]); + + function buildContext(varName: string): Record { + const ctx: Record = { ...variablesRef.current }; + delete ctx[varName]; + return ctx; + } + function handleVariableChange(varName: string, value: string) { - setVariables((prev) => { - const next = { ...prev, [varName]: value }; - if (useAutocomplete) { - // Schedule a debounced completion call. The `context` carries the - // other variables' current values so the server can disambiguate - // when one variable depends on another. - const context: Record = { ...next }; - delete context[varName]; - const existing = timersRef.current.get(varName); - if (existing) clearTimeout(existing); - const timer = setTimeout(() => { - timersRef.current.delete(varName); - void runCompletion(varName, value, context); - }, COMPLETION_DEBOUNCE_MS); - timersRef.current.set(varName, timer); - } - return next; - }); + setVariables((prev) => ({ ...prev, [varName]: value })); + if (!useAutocomplete) return; + const existing = timersRef.current.get(varName); + if (existing) clearTimeout(existing); + const timer = setTimeout(() => { + timersRef.current.delete(varName); + // Build context at fire time so sibling updates that arrived + // between schedule and fire are picked up. + void runCompletion(varName, value, buildContext(varName)); + }, COMPLETION_DEBOUNCE_MS); + timersRef.current.set(varName, timer); } function handleVariableFocus(varName: string) { @@ -196,10 +204,8 @@ export function ResourceTemplatePanel({ clearTimeout(existing); timersRef.current.delete(varName); } - const value = variables[varName] ?? ""; - const context: Record = { ...variables }; - delete context[varName]; - void runCompletion(varName, value, context); + const value = variablesRef.current[varName] ?? ""; + void runCompletion(varName, value, buildContext(varName)); } const canSubmit = variableNames.every((n) => variables[n]?.length > 0); diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx index 482fed2a7..025b16ef6 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.test.tsx @@ -186,6 +186,24 @@ describe("PromptsScreen", () => { expect(screen.queryByText("Messages")).not.toBeInTheDocument(); }); + it("re-clicking the active prompt preserves form values and does not re-fetch", async () => { + const user = userEvent.setup(); + const onGetPrompt = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByText("ping")); + expect(onGetPrompt).toHaveBeenCalledTimes(1); + // Sidebar re-click on the same prompt should be a no-op — sidebar + // is navigation, ✕ is dismiss. + await user.click(screen.getByText("ping")); + expect(onGetPrompt).toHaveBeenCalledTimes(1); + }); + it("resets argument values when switching prompts", async () => { const user = userEvent.setup(); const arglessTwoStep: Prompt[] = [ diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index 6b82373b9..bc56f17e8 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -127,6 +127,11 @@ export function PromptsScreen({ : undefined; function handleSelectPrompt(name: string) { + // Re-clicking the active prompt in the sidebar shouldn't wipe the + // user's typed argument values or trigger a re-fetch — sidebar is + // for navigation, ✕ is for dismiss. Closing-then-reselecting is + // its own thing (the close handler clears submittedFor). + if (name === selectedPromptName) return; setArgumentValues({}); setSelectedPromptName(name); // Auto-fetch no-argument prompts the moment they're selected — the From 9c1865431d07eacd92e7a7b422ad4355f3f43ed8 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 23 May 2026 09:09:33 -0400 Subject: [PATCH 9/9] Address follow-up review nits: ghost suggestions, dead previewActive branch - Clear `completions[name]` at the top of handleChange in both PromptArgumentsForm and ResourceTemplatePanel so the dropdown doesn't show stale options from the previous prefix during the 300ms debounce + network window. - Simplify previewActive in PromptsScreen to drop the unreachable `!getPromptState?.promptName` fallback. App.tsx tags every prompt state transition with `promptName`, so the fallback was dead code that obscured the actual runtime invariant. - Add a focused test for each form that asserts stale dropdown options vanish the instant a new keystroke arrives. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PromptArgumentsForm.test.tsx | 39 +++++++++++++++++++ .../PromptArgumentsForm.tsx | 10 +++++ .../ResourceTemplatePanel.test.tsx | 39 +++++++++++++++++++ .../ResourceTemplatePanel.tsx | 9 +++++ .../screens/PromptsScreen/PromptsScreen.tsx | 12 +++--- 5 files changed, 104 insertions(+), 5 deletions(-) diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx index 99f55ddc6..76ef5942c 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx @@ -337,6 +337,45 @@ describe("PromptArgumentsForm", () => { expect(textCalls.at(-1)).toEqual(["text", "h", { targetLanguage: "es" }]); }); + it("clears stale dropdown options the instant a new keystroke arrives", async () => { + const user = userEvent.setup(); + const deferred: Array<{ + value: string; + resolve: (values: string[]) => void; + }> = []; + const onCompleteArgument = vi.fn( + (_argName: string, value: string) => + new Promise((resolve) => { + deferred.push({ value, resolve }); + }), + ); + + renderWithMantine( + , + ); + + // Focus → first call (value=""). Resolve so the dropdown has + // something to show. + await user.click(screen.getByRole("textbox", { name: /^text/ })); + await new Promise((r) => setTimeout(r, 0)); + expect(deferred.length).toBe(1); + deferred[0].resolve(["alpha", "alphabet"]); + expect(await screen.findByText("alpha")).toBeInTheDocument(); + + // Type a new character — the keystroke handler must drop the + // stale options immediately so the dropdown doesn't show + // "alpha" / "alphabet" while the next request is in flight + // (300ms debounce + network latency). + await user.type(screen.getByRole("textbox", { name: /^text/ }), "z"); + expect(screen.queryByText("alpha")).not.toBeInTheDocument(); + expect(screen.queryByText("alphabet")).not.toBeInTheDocument(); + }); + it("aborts an in-flight request when a faster keystroke arrives", async () => { const user = userEvent.setup(); const calls: Array<{ diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx index f93f51b33..10c1a0b62 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx @@ -143,6 +143,16 @@ export function PromptArgumentsForm({ function handleChange(argName: string, value: string) { onArgumentChange(argName, value); if (!useAutocomplete) return; + // Drop the previous prefix's completions so the dropdown doesn't + // show ghost suggestions from the old keystroke while the new + // request is in flight (300ms debounce + network latency). The + // fresh response repopulates the array when it arrives. + setCompletions((prev) => { + if (prev[argName] === undefined) return prev; + const next = { ...prev }; + delete next[argName]; + return next; + }); const existing = timersRef.current.get(argName); if (existing) clearTimeout(existing); const timer = setTimeout(() => { diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx index 7005c5bfe..d449cdbfa 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx @@ -245,6 +245,45 @@ describe("ResourceTemplatePanel", () => { }); }); + it("clears stale dropdown options the instant a new keystroke arrives", async () => { + const user = userEvent.setup(); + const deferred: Array<{ + value: string; + resolve: (values: string[]) => void; + }> = []; + const onCompleteArgument = vi.fn( + (_argName: string, value: string) => + new Promise((resolve) => { + deferred.push({ value, resolve }); + }), + ); + + renderWithMantine( + , + ); + + // Focus → first call (value=""). Resolve so the dropdown has + // something to show. + await user.click(screen.getByRole("textbox", { name: "userId" })); + await new Promise((r) => setTimeout(r, 0)); + expect(deferred.length).toBe(1); + deferred[0].resolve(["alpha", "alphabet"]); + expect(await screen.findByText("alpha")).toBeInTheDocument(); + + // Type a new character — the keystroke handler must drop the + // stale options immediately so the dropdown doesn't show + // "alpha" / "alphabet" while the next request is in flight + // (300ms debounce + network latency). + await user.type(screen.getByRole("textbox", { name: "userId" }), "z"); + expect(screen.queryByText("alpha")).not.toBeInTheDocument(); + expect(screen.queryByText("alphabet")).not.toBeInTheDocument(); + }); + it("does not call onCompleteArgument when completions are unsupported", async () => { const user = userEvent.setup(); const onCompleteArgument = vi.fn(); diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx index 89e672d00..3064d7390 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx @@ -181,6 +181,15 @@ export function ResourceTemplatePanel({ function handleVariableChange(varName: string, value: string) { setVariables((prev) => ({ ...prev, [varName]: value })); if (!useAutocomplete) return; + // Drop the previous prefix's completions so the dropdown doesn't + // show ghost suggestions from the old keystroke while the new + // request is in flight (300ms debounce + network latency). + setCompletions((prev) => { + if (prev[varName] === undefined) return prev; + const next = { ...prev }; + delete next[varName]; + return next; + }); const existing = timersRef.current.get(varName); if (existing) clearTimeout(existing); const timer = setTimeout(() => { diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index bc56f17e8..a3d7f1aba 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -166,14 +166,16 @@ export function PromptsScreen({ } // The preview is "active" when we've submitted (or auto-fetched) the - // currently-selected prompt and the parent's result/pending/error state - // matches. Without the name check, a stale result from a previous - // prompt would leak into the new prompt's pane. + // currently-selected prompt and the parent's state is tagged with + // the matching prompt name. The name match guards against a stale + // result from a previously-selected prompt leaking into the new + // prompt's pane. App.tsx tags every state transition with + // `promptName`, so we don't need a fallback for untagged states. const previewActive = !!selectedPrompt && + !!getPromptState && submittedFor === selectedPrompt.name && - (!getPromptState?.promptName || - getPromptState.promptName === selectedPrompt.name); + getPromptState.promptName === selectedPrompt.name; function renderPreview() { if (!previewActive || !getPromptState) return null;