Address review feedback: canSubmit, fire-time context, re-click guard

- 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) <noreply@anthropic.com>

Author: cliffhall

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;

clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx

@@ -150,7 +150,8 @@ describe("PromptArgumentsForm", () => {
     renderWithMantine(
       <PromptArgumentsForm
         prompt={promptWithArgs}
-        argumentValues={{}}
+        // Required arg filled — otherwise the button is disabled.
+        argumentValues={{ text: "Hello" }}
         onArgumentChange={vi.fn()}
         onGetPrompt={onGetPrompt}
       />,
@@ -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(
+      <StatefulForm prompt={promptWithArgs} onGetPrompt={vi.fn()} />,
+    );
+    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(
+      <PromptArgumentsForm
+        prompt={promptWithArgs}
+        argumentValues={{ text: "Hello" }}
+        onArgumentChange={vi.fn()}
+        onGetPrompt={vi.fn()}
+      />,
+    );
+    // 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(
       <PromptArgumentsForm
@@ -267,6 +295,92 @@ describe("PromptArgumentsForm", () => {
       });
     });
 
+    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<string, string>,
+          ) => Promise<string[]>
+        >()
+        .mockResolvedValue([]);
+
+      renderWithMantine(
+        <StatefulForm
+          prompt={promptWithArgs}
+          onGetPrompt={vi.fn()}
+          completionsSupported
+          onCompleteArgument={onCompleteArgument}
+        />,
+      );
+
+      // 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<string[]>((resolve) => {
+            calls.push({ value, resolve });
+          }),
+      );
+
+      renderWithMantine(
+        <StatefulForm
+          prompt={promptWithArgs}
+          onGetPrompt={vi.fn()}
+          completionsSupported
+          onCompleteArgument={onCompleteArgument}
+        />,
+      );
+
+      // 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();

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<string, string> {
-    const ctx: Record<string, string> = {};
-    for (const a of promptArguments ?? []) {
-      if (a.name === currentArg) continue;
-      ctx[a.name] = argumentValues[a.name] ?? "";
-    }
-    return ctx;
-  }
+  const buildContext = useCallback(
+    (currentArg: string): Record<string, string> => {
+      const ctx: Record<string, string> = {};
+      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 (
     <Stack gap="md">
       <PromptTitle>{title ?? name}</PromptTitle>
@@ -198,7 +218,7 @@ export function PromptArgumentsForm({
         </>
       )}
       <Group justify="flex-end">
-        <Button size="sm" onClick={onGetPrompt}>
+        <Button size="sm" disabled={!canSubmit} onClick={onGetPrompt}>
           Get Prompt
         </Button>
       </Group>

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<string, string> {
+    const ctx: Record<string, string> = { ...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<string, string> = { ...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<string, string> = { ...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);

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(
+      <PromptsScreen
+        {...baseProps}
+        prompts={noArgPrompts}
+        onGetPrompt={onGetPrompt}
+      />,
+    );
+    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[] = [

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