Merge pull request #2329 from modelcontextprotocol/v2/fix/2328-connection-info-labels

fix: bold the labels, not the values, and give Connection Info's URLs full width

clients/web/src/components/elements/EraBadge/EraBadge.tsx

@@ -5,14 +5,22 @@ import { formatEra, isModernEra } from "./eraUtils";
 export interface EraBadgeProps {
   /** The negotiated protocol era; `undefined` renders as Legacy. */
   era: ProtocolEra | undefined;
+  /**
+   * Font weight override. The app-wide `ThemeBadge` defaults to `fw: 600`,
+   * which is right where this badge is a standalone chip (ProtocolListPanel's
+   * header). Connection Info renders it as the *value* half of a label/value
+   * row, where the convention is bold label / normal value, so it passes 400.
+   * Left undefined the theme default applies (#2328).
+   */
+  fw?: number;
 }
 
 // Labels a connection's negotiated protocol era (SEP §7.8). Feed it from
 // connection state only — see the note in `eraUtils` on why the era must never
 // be inferred from individual message frames.
-export function EraBadge({ era }: EraBadgeProps) {
+export function EraBadge({ era, fw }: EraBadgeProps) {
   return (
-    <Badge variant="outline" color={isModernEra(era) ? "blue" : "gray"}>
+    <Badge variant="outline" color={isModernEra(era) ? "blue" : "gray"} fw={fw}>
       {formatEra(era)}
     </Badge>
   );

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

@@ -567,6 +567,168 @@ describe("ConnectionInfoContent", () => {
     expect(screen.getByText("token-123")).toBeInTheDocument();
   });
 
+  it("weights labels bold and values normal, in both halves of the modal", () => {
+    renderWithMantine(
+      <ConnectionInfoContent
+        initializeResult={fullResult}
+        clientCapabilities={fullClientCaps}
+        transport="streamable-http"
+        oauth={{
+          protocol: "standard",
+          authorized: true,
+          clientId: "client-abc",
+          scopes: ["read"],
+        }}
+      />,
+    );
+
+    // The convention is the whole point of #2328: the label is the fixed
+    // scaffolding a reader scans down, the value is what differs. Asserted in
+    // Server Implementation *and* OAuth Details, because the defect being
+    // guarded against is the two halves disagreeing — checking one alone would
+    // pass on a modal that is internally inconsistent.
+    for (const label of ["Name", "Protocol", "Client ID", "Scopes"]) {
+      expect(screen.getAllByText(label)[0]).toHaveStyle({ fontWeight: "600" });
+    }
+    // `getByText`, not `queryByText` + `?? ""`: an absent value would make the
+    // optional form pass vacuously (undefined → "" → not "600"), so the test
+    // would go green on a row that had stopped rendering at all.
+    for (const value of ["Everything Server", "read"]) {
+      expect(screen.getByText(value).style.fontWeight).not.toBe("600");
+    }
+
+    // Badge values count too. `ThemeBadge` defaults to `fw: 600`, so Status,
+    // Transport and Era read bold-label/bold-value unless overridden — the
+    // whole-modal claim is false without this.
+    for (const badge of ["streamable-http", "Legacy", "Authorized"]) {
+      // `getByText` lands on the Badge's inner label span; `fw` is applied to
+      // the root, so walk up to it or the assertion reads an empty string and
+      // passes against anything.
+      const root = screen.getByText(badge).closest('[class*="Badge-root"]');
+      expect((root as HTMLElement | null)?.style.fontWeight).toBe("400");
+    }
+  });
+
+  it("gives Client ID and Auth URL the full width, with no inset surface", () => {
+    renderWithMantine(
+      <ConnectionInfoContent
+        initializeResult={fullResult}
+        clientCapabilities={fullClientCaps}
+        transport="streamable-http"
+        oauth={{
+          protocol: "standard",
+          authorized: true,
+          clientId:
+            "http://127.0.0.1:8093/client-metadata.json?profile=long-enough-to-wrap",
+          authUrl: "https://auth.example.com/authorize",
+        }}
+      />,
+    );
+
+    // A CIMD client id IS a URL, so it is long by construction. In the
+    // two-column grid it got half the modal and broke mid-token
+    // (`…/client-metadata.` / `json`), which reads as a rendering fault rather
+    // than as one value.
+    for (const value of [
+      "http://127.0.0.1:8093/client-metadata.json?profile=long-enough-to-wrap",
+      "https://auth.example.com/authorize",
+    ]) {
+      expect(screen.getByText(value)).toHaveStyle({
+        backgroundColor: "transparent",
+      });
+    }
+
+    // The background alone does not pin the *layout*: swapping FullWidthField
+    // back for the two-column SimpleGrid would keep it transparent and still
+    // pass. Assert the structure that makes the value full width — label and
+    // value are siblings in a column, and neither sits in a SimpleGrid.
+    for (const [label, value] of [
+      [
+        "Client ID",
+        "http://127.0.0.1:8093/client-metadata.json?profile=long-enough-to-wrap",
+      ],
+      ["Auth URL", "https://auth.example.com/authorize"],
+    ]) {
+      const labelNode = screen.getByText(label);
+      const valueNode = screen.getByText(value);
+      expect(valueNode.parentElement).toBe(labelNode.parentElement);
+      expect(valueNode.closest('[class*="SimpleGrid"]')).toBeNull();
+    }
+  });
+
+  it("bolds the token captions, which are field labels in the same list", () => {
+    renderWithMantine(
+      <ConnectionInfoContent
+        initializeResult={fullResult}
+        clientCapabilities={fullClientCaps}
+        transport="streamable-http"
+        oauth={{
+          protocol: "standard",
+          authorized: true,
+          accessToken: "token-123",
+          idToken: "id-token-456",
+        }}
+      />,
+    );
+
+    // OAuthTokenField renders its own caption, so the weight convention has to
+    // be asserted through it — the other weight test renders no token at all.
+    for (const caption of ["Access Token", "ID Token"]) {
+      expect(screen.getByText(caption).style.fontWeight).toBe("600");
+    }
+  });
+
+  it("groups the full-width fields at the end, Client ID directly above Access Token", () => {
+    renderWithMantine(
+      <ConnectionInfoContent
+        initializeResult={fullResult}
+        clientCapabilities={fullClientCaps}
+        transport="streamable-http"
+        oauth={{
+          protocol: "standard",
+          authorized: true,
+          clientId: "http://127.0.0.1:8093/client-metadata.json",
+          clientRegistrationKind: "cimd",
+          authUrl: "https://auth.example.com/authorize",
+          scopes: ["mcp"],
+          accessToken: "token-123",
+        }}
+      />,
+    );
+
+    // Order is the assertion, so read the labels off the DOM rather than
+    // checking each is merely present: the inline two-column rows come first,
+    // then every label-over-value field together. Interleaving them is what
+    // this guards against — it broke the scan down the label column, and the
+    // tokens already used the full-width layout at the bottom.
+    const order = [
+      "Client registration",
+      "Scopes",
+      "Auth URL",
+      "Client ID",
+      "Access Token",
+    ];
+    const positions = order.map((label) => {
+      const node = screen.getAllByText(label)[0];
+      expect(node).toBeInTheDocument();
+      return (
+        node.compareDocumentPosition(screen.getAllByText("Protocol")[0]) &
+        Node.DOCUMENT_POSITION_PRECEDING
+      );
+    });
+    expect(positions.every(Boolean)).toBe(true);
+
+    const labelNode = (label: string) => screen.getAllByText(label)[0];
+    for (let i = 0; i < order.length - 1; i++) {
+      const earlier = labelNode(order[i]);
+      const later = labelNode(order[i + 1]);
+      expect(
+        earlier.compareDocumentPosition(later) &
+          Node.DOCUMENT_POSITION_FOLLOWING,
+      ).toBeTruthy();
+    }
+  });
+
   it("renders EMA idp session when provided", () => {
     renderWithMantine(
       <ConnectionInfoContent

clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx

@@ -79,11 +79,26 @@ export interface ConnectionInfoContentProps {
   onClearOAuth?: () => void;
 }
 
-const ValueText = Text.withProps({
+// Label/value pairs in this modal read label-bold, value-normal: the label is
+// the fixed scaffolding a reader scans down, and the value is the thing that
+// differs per connection. The reverse (which this was until #2328) bolded every
+// answer, so nothing stood out and the two columns fought each other.
+const FieldLabel = Text.withProps({
   size: "sm",
   fw: 600,
 });
 
+const ValueText = Text.withProps({
+  size: "sm",
+});
+
+// A badge standing in as the *value* half of a label/value row. The app-wide
+// `ThemeBadge` defaults to `fw: 600`, which is right for a standalone chip but
+// makes these rows read bold-label/bold-value — the one convention this modal
+// is not supposed to have. The chip still reads as a chip: its emphasis comes
+// from the outline and colour, not the font weight (#2328).
+const ValueBadge = Badge.withProps({ variant: "outline", fw: 400 });
+
 // Shown for Name/Version when the server didn't report `serverInfo` — an em dash
 // plus an explicit note so the client-side catalog fallback is never mistaken
 // for a value the server sent. Exported so tests/stories assert against it
@@ -104,7 +119,14 @@ const SectionHeading = Title.withProps({
 // `Code` block — that keeps the whole value visible and removes a scroll region
 // that would otherwise need its own keyboard access (axe
 // `scrollable-region-focusable`).
-const ValueCode = Code.withProps({ variant: "wrapping" });
+const ValueCode = Code.withProps({ variant: "wrapping-plain" });
+
+// A long OAuth value (client id, auth URL) gets its label on its own line and
+// the value across the full modal width beneath it, the way `OAuthTokenField`
+// already lays out a token. In the two-column grid these values had roughly
+// half the width and wrapped mid-token — `…/client-metadata.` / `json` — which
+// reads as a rendering fault rather than as one URL (#2328).
+const FullWidthField = Stack.withProps({ gap: 4 });
 
 // One declared sub-option of an extension. Mirrors `CapabilityItem`'s ✓/✗ row
 // rather than reusing it: that element's `capability` prop is the closed union
@@ -306,22 +328,22 @@ export function ConnectionInfoContent({
       <Stack gap="xs">
         <SectionHeading>Server Implementation</SectionHeading>
         <SimpleGrid cols={2}>
-          <Text size="sm">Name</Text>
+          <FieldLabel>Name</FieldLabel>
           <ValueText>{displayName}</ValueText>
 
-          <Text size="sm">Version</Text>
+          <FieldLabel>Version</FieldLabel>
           <ValueText>{displayVersion}</ValueText>
 
-          <Text size="sm">Protocol</Text>
+          <FieldLabel>Protocol</FieldLabel>
           <ValueText>{protocolVersion || "—"}</ValueText>
 
-          <Text size="sm">Transport</Text>
+          <FieldLabel>Transport</FieldLabel>
-          <Badge variant="outline">{transport}</Badge>
+          <ValueBadge>{transport}</ValueBadge>
 
-          <Text size="sm">Era</Text>
+          <FieldLabel>Era</FieldLabel>
-          <EraBadge era={protocolEra} />
+          <EraBadge era={protocolEra} fw={400} />
 
-          <Text size="sm">Session</Text>
+          <FieldLabel>Session</FieldLabel>
           <ValueText>{formatSession(protocolEra, transport)}</ValueText>
         </SimpleGrid>
       </Stack>
@@ -330,7 +352,7 @@ export function ConnectionInfoContent({
         <Stack gap="xs">
           <SectionHeading>Discovery</SectionHeading>
           <SimpleGrid cols={2}>
-            <Text size="sm">Supported versions</Text>
+            <FieldLabel>Supported versions</FieldLabel>
             <ValueText>
               {discoverResult.supportedVersions.length > 0
                 ? discoverResult.supportedVersions.join(", ")
@@ -433,26 +455,17 @@ export function ConnectionInfoContent({
           <SectionHeading>OAuth Details</SectionHeading>
           <Stack gap="xs">
             <SimpleGrid cols={2}>
-              <Text size="sm">Protocol</Text>
+              <FieldLabel>Protocol</FieldLabel>
               <ValueText>{formatProtocol(oauth.protocol)}</ValueText>
 
-              <Text size="sm">Status</Text>
+              <FieldLabel>Status</FieldLabel>
-              <Badge
+              <ValueBadge color={oauth.authorized ? "green" : "gray"}>
-                variant="outline"
-                color={oauth.authorized ? "green" : "gray"}
-              >
                 {oauth.authorized ? "Authorized" : "Not authorized"}
-              </Badge>
+              </ValueBadge>
             </SimpleGrid>
-            {oauth.clientId && (
-              <SimpleGrid cols={2}>
-                <Text size="sm">Client ID</Text>
-                <ValueCode>{oauth.clientId}</ValueCode>
-              </SimpleGrid>
-            )}
             {oauth.clientRegistrationKind && (
               <SimpleGrid cols={2}>
-                <Text size="sm">Client registration</Text>
+                <FieldLabel>Client registration</FieldLabel>
                 <ValueText>
                   {formatClientRegistrationKind(oauth.clientRegistrationKind)}
                 </ValueText>
@@ -460,22 +473,34 @@ export function ConnectionInfoContent({
             )}
             {oauth.protocol === "ema" && oauth.idpSession && (
               <SimpleGrid cols={2}>
-                <Text size="sm">IdP session</Text>
+                <FieldLabel>IdP session</FieldLabel>
                 <ValueText>{formatIdpSession(oauth.idpSession)}</ValueText>
               </SimpleGrid>
             )}
-            {oauth.authUrl && (
-              <SimpleGrid cols={2}>
-                <Text size="sm">Auth URL</Text>
-                <ValueCode>{oauth.authUrl}</ValueCode>
-              </SimpleGrid>
-            )}
             {oauth.scopes && oauth.scopes.length > 0 && (
               <SimpleGrid cols={2}>
-                <Text size="sm">Scopes</Text>
+                <FieldLabel>Scopes</FieldLabel>
                 <ValueText>{formatScopes(oauth.scopes)}</ValueText>
               </SimpleGrid>
             )}
+            {/* The full-width fields are kept together at the end of the
+                section, directly above the token rows, which use the same
+                label-over-value layout. Interleaved with the inline two-column
+                rows they broke the scan down the label column for every row
+                after them, and Client ID — the one most likely to be long —
+                was the worst offender (#2328). */}
+            {oauth.authUrl && (
+              <FullWidthField>
+                <FieldLabel>Auth URL</FieldLabel>
+                <ValueCode>{oauth.authUrl}</ValueCode>
+              </FullWidthField>
+            )}
+            {oauth.clientId && (
+              <FullWidthField>
+                <FieldLabel>Client ID</FieldLabel>
+                <ValueCode>{oauth.clientId}</ValueCode>
+              </FullWidthField>
+            )}
             {oauth.accessToken && (
               <OAuthTokenField
                 label="Access Token"

clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx

@@ -26,7 +26,10 @@ const CaptionRow = Flex.withProps({
   wrap: "nowrap",
 });
 
-const Caption = Text.withProps({ size: "sm" });
+// Bold, matching every other field label in Connection Info (#2328) — the
+// token rows sit in the same OAuth Details list as Client ID and Scopes, so a
+// lighter caption here would break the column a reader scans down.
+const Caption = Text.withProps({ size: "sm", fw: 600 });
 
 const Toolbar = Flex.withProps({
   gap: 4,

clients/web/src/theme/Code.test.tsx

@@ -0,0 +1,60 @@
+import { describe, it, expect } from "vitest";
+import { Code } from "@mantine/core";
+import { renderWithMantine } from "../test/renderWithMantine";
+
+// `ThemeCode.extend` is applied transitively wherever a Code renders, but no
+// single component exercises every branch of its styles callback. Same reason
+// as `Paper.test.tsx` (#1787 brought theme/** under the coverage gate).
+//
+// The load-bearing assertion here is the `wrapping` / `wrapping-plain` split
+// (#2328): `wrapping-plain` exists so Connection Info can drop the inset
+// surface behind a label/value pair *without* changing `wrapping`, which
+// ElicitationUrlPanel, ListLoadError, MalformedItemsWarning and the CSV
+// fallback all rely on to separate a raw value from the prose around it.
+
+describe("ThemeCode variants", () => {
+  it("renders the default variant", () => {
+    const { getByText } = renderWithMantine(<Code>default</Code>);
+    expect(getByText("default")).toBeTruthy();
+  });
+
+  it("wraps long values in both wrapping variants", () => {
+    for (const variant of ["wrapping", "wrapping-plain"] as const) {
+      const { getByText } = renderWithMantine(
+        <Code variant={variant}>{variant}</Code>,
+      );
+      const style = getByText(variant).style;
+      expect(style.wordBreak).toBe("break-all");
+      expect(style.whiteSpace).toBe("pre-wrap");
+    }
+  });
+
+  it("strips the inset surface for wrapping-plain only", () => {
+    const { getByText: getPlain } = renderWithMantine(
+      <Code variant="wrapping-plain">plain</Code>,
+    );
+    expect(getPlain("plain").style.backgroundColor).toBe("transparent");
+    expect(getPlain("plain").style.padding).toBe("0px");
+
+    // The guard that makes this change safe: `wrapping` keeps its surface, so
+    // every other consumer of it is untouched.
+    const { getByText: getInset } = renderWithMantine(
+      <Code variant="wrapping">inset</Code>,
+    );
+    expect(getInset("inset").style.backgroundColor).not.toBe("transparent");
+    expect(getInset("inset").style.padding).not.toBe("0px");
+  });
+
+  it("clips instead of wrapping for the nowrap variant", () => {
+    const { getByText } = renderWithMantine(<Code variant="nowrap">n</Code>);
+    const style = getByText("n").style;
+    expect(style.whiteSpace).toBe("nowrap");
+    expect(style.overflow).toBe("hidden");
+    expect(style.textOverflow).toBe("ellipsis");
+  });
+
+  it("drops the margin on a block Code", () => {
+    const { getByText } = renderWithMantine(<Code block>b</Code>);
+    expect(getByText("b").style.margin).toBe("0px");
+  });
+});

clients/web/src/theme/Code.ts

@@ -10,10 +10,20 @@ export const ThemeCode = Code.extend({
       backgroundColor:
         "var(--inspector-code-surface, var(--inspector-surface-code))",
     };
-    if (props.variant === "wrapping") {
+    if (props.variant === "wrapping" || props.variant === "wrapping-plain") {
       root.wordBreak = "break-all";
       root.whiteSpace = "pre-wrap";
     }
+    if (props.variant === "wrapping-plain") {
+      // Wraps like `wrapping`, but with no inset surface behind it. Used for
+      // label/value rows that read as *values* rather than as quoted blobs —
+      // Connection Info's Client ID and Auth URL. A separate variant rather
+      // than a tweak to `wrapping`, because that one is shared with panels
+      // (elicitation URLs, load errors, CSV fallback) where the grey inset is
+      // doing real work separating a raw value from the prose around it.
+      root.backgroundColor = "transparent";
+      root.padding = "0";
+    }
     if (props.block) {
       root.margin = "0";
     }