refactor(web): sweep AGENTS.md TypeScript + Mantine rule violations (#1762)

AGENTS.md

@@ -251,8 +251,9 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i
   - NEVER use inline code; instead extract to functions in the same file, exported or located in a shared location if immediately reusable.
   - In a component's file, for sub-components:
     - ALWAYS use Mantine components for layout and content, configured with props for styling and behavior.
-    - ALWAYS declare a meaningfully named subcomponent as a constant using `.withProps()` if a component has two or more props.
-    - NEVER use `Box` for subcomponent constants — `Box` does not support `.withProps()`. Use `Group`, `Stack`, `Flex`, `Text`, `Paper`, `UnstyledButton`, or `Image` instead. Pick the component that best matches the purpose: `Paper` for bordered/surfaced containers, `Text` for any text or content wrapper, `Stack`/`Group`/`Flex` for layout.
+    - ALWAYS declare a meaningfully named subcomponent as a constant using `.withProps()` if an inline Mantine element carries two or more **static** props. A *static* prop is one whose value is a literal that configures the element's **styling, layout, or behavior** (`size="sm"`, `c="dimmed"`, `fw={500}`, `gap="xs"`, `justify="space-between"`, `variant="light"`, `withBorder`, `readOnly`, `striped`, …); dynamic props (`value`, `onChange`/`on*`, `children`, `key`, `ref`, and anything whose value is a variable/expression) do **not** count toward the two and are passed at the call site, not baked into the constant. Purely per-instance **content/accessibility** literals — `label`, `description`, `placeholder`, `title`, `aria-label`, `role` — likewise do **not** count toward the two (a `<Checkbox label="…" description="…">` with no styling/layout/behavior props stays inline); they may be baked into a constant when it already qualifies and doing so aids reuse, but they never by themselves trigger extraction. This rule applies in **all** cases: "repeated pattern" is NOT the bar — a single-use element with two or more static styling/layout/behavior props must still be extracted. Bake the static props into the `.withProps()` constant and pass the dynamic ones where it's rendered.
+    - The following **cannot** be expressed via `.withProps()` and so stay inline (like `Box` below), each with a one-line comment saying why: **`Accordion`** (a compound, `multiple`-discriminated generic — `.withProps({ multiple: true, … })` loses its JSX call signature and fails to type); **headless, non-`factory()` Mantine components** such as **`Transition`** (plain function components with no Styles API — they have no `.withProps` static at all, e.g. `Transition.withProps` is a TS2339); and **`data-*` attributes** (not part of a component's typed props object, so excess-property-checked out of a `withProps` literal — pass them at the call site). The rule targets factory-based (Styles-API) Mantine components; anything that isn't one is out of scope entirely — a third-party element (a `react-icons` glyph, another library's component) **and** a first-party component that isn't a Mantine factory (a dumb `export function` like `ContentViewer`, which has no `.withProps` static of its own).
+    - NEVER use `Box` for subcomponent constants — `Box` does not support `.withProps()`. Use `Group`, `Stack`, `Flex`, `Text`, `Paper`, `UnstyledButton`, or `Image` instead. Pick the component that best matches the purpose: `Paper` for bordered/surfaced containers, `Text` for any text or content wrapper, `Stack`/`Group`/`Flex` for layout. A `Box` that genuinely needs a non-flex primitive it can't provide — `component="iframe"`, or `display="grid"` (no Mantine flex primitive is a CSS grid) — stays a `Box` inline, with a one-line comment saying why.
     - NEVER use a CSS class on a subcomponent constant when the styles can be expressed as a Mantine theme variant instead. Define variants in `src/theme/<Component>.ts` using `Component.extend({ styles: (_theme, props) => { ... } })` and reference them with `variant="variantName"` on the component or in `.withProps()`.
     - CSS classes are ONLY acceptable on subcomponents for styles that cannot be expressed as flat CSS-in-JS properties in the theme — specifically: pseudo-selectors (`:hover`, `:focus`), cross-component hover relationships (`.parent:hover .child`), nested child-element selectors (`.wrapper p`, `.wrapper code`), `@keyframes` definitions, and native HTML elements (`img`, `iframe`) that are not Mantine components.
     - When a theme variant needs a CSS class for nested/pseudo selectors, use `classNames` in the theme extension to auto-assign it — never add `className` manually in JSX for theme-styled components.

clients/web/src/App.css

@@ -266,15 +266,6 @@ body {
   }
 }
 
-@keyframes inspector-spin {
-  from {
-    transform: rotate(0deg);
-  }
-  to {
-    transform: rotate(360deg);
-  }
-}
-
 /* Both motions travel downward: enter descends from above into place, exit
    descends from place to below — used for the header crossfade (#1450). */
 @keyframes inspector-fade-slide-in {
@@ -318,10 +309,6 @@ body {
   animation: inspector-pulse 1.5s ease-in-out infinite;
 }
 
-.inspector-spin {
-  animation: inspector-spin 1s linear infinite;
-}
-
 /* Applied (via the `tabGlow` Text variant) to a tab label; the glow starts full
    and fades, running only while `data-glow="on"` on a freshly-appeared tab. */
 .tab-glow[data-glow="on"] {
@@ -343,13 +330,6 @@ body {
   animation: inspector-fade-slide-out 300ms ease both;
 }
 
-/* The title and tab bar additionally share one grid cell so one replaces the
-   other in place during the center crossfade. */
-.header-stack-cell {
-  grid-area: 1 / 1;
-  place-self: center;
-}
-
 /* Honor reduced-motion for the header animations this PR adds (#1450): skip the
    fade/slide and the new-tab glow, leaving the elements at their resting state.
    Selectors match the originals' specificity so they win inside the query. */
@@ -528,12 +508,6 @@ body {
   background-color: var(--mantine-primary-color-light-hover);
 }
 
-/* ── Grid alignment ─────────────────────────────────────── */
-
-.grid-align-start {
-  align-items: start;
-}
-
 /* ── Markdown content (third-party HTML from react-markdown) ───── */
 
 .markdown-content {
@@ -583,16 +557,6 @@ body {
   cursor: grabbing;
 }
 
-/*
- * Sticky re-auth banner bar. Positioning (pos/top) and background are set via
- * Mantine props on the Box; z-index and box-shadow have no Mantine style-prop
- * shorthand, so they live here.
- */
-.reauth-banner-bar {
-  z-index: 200;
-  box-shadow: var(--mantine-shadow-sm);
-}
-
 /*
  * Draggable vertical divider between the primary screen area and the pinned
  * monitoring column (#1616). The col-resize cursor, the hover/active accent,

clients/web/src/App.tsx

@@ -3,6 +3,7 @@ import {
   Anchor,
   Box,
   List,
+  Paper,
   Stack,
   Text,
   useComputedColorScheme,
@@ -270,10 +271,12 @@ function messagesToLogEntries(messages: MessageEntry[]): LogEntryData[] {
   for (const m of messages) {
     if (m.direction !== "notification") continue;
     // MessageEntry.message is a JSONRPC union; notifications have `method`
-    // but not `id`. Narrow with an `in` check before the cast.
+    // but not `id`. Narrow with an `in` check, then confirm the method.
     if (!("method" in m.message)) continue;
     if (m.message.method !== "notifications/message") continue;
-    const params = (m.message as unknown as LoggingMessageNotification).params;
+    // The method check pins this to a logging notification; its `params` are
+    // only generically typed on the JSONRPC union, so cast just that value.
+    const params = m.message.params as LoggingMessageNotification["params"];
     out.push({
       receivedAt: m.timestamp,
       params,
@@ -381,6 +384,34 @@ function bodyDroppedToastId(serverId: string): string {
 
 const CLIENT_CONFIG_LOAD_ERROR_NOTIFICATION_ID = "client-config-load-error";
 
+// Shared "list of likely causes" styling for the warning-toast bodies below.
+const ToastCauseList = List.withProps({ size: "sm", spacing: 2 });
+
+// The "open the relevant settings/details" link rendered at the bottom of each
+// warning-toast body. Same static shape across all three toasts; each passes
+// its own `onClick`.
+const ToastLinkButton = Anchor.withProps({
+  component: "button",
+  type: "button",
+  size: "sm",
+});
+
+// Sticky re-auth banner bar. A `Paper` so every static style is a prop: `shadow`
+// emits `var(--mantine-shadow-sm)` (identical to the old CSS), and the stacking
+// order goes through `styles.root` since Mantine has no `z` prop.
+const ReAuthBannerBar = Paper.withProps({
+  px: "md",
+  pt: "xs",
+  pos: "sticky",
+  top: 60,
+  bg: "var(--mantine-color-body)",
+  shadow: "sm",
+  styles: { root: { zIndex: 200 } },
+  // Paper's default `radius: "md"` would round this full-bleed sticky bar's
+  // corners; the bar it replaced (a Box) had none.
+  radius: 0,
+});
+
 // Body of the "response body dropped" warning toast: a one-line summary of what
 // happened, the likely causes, and a link that opens this server's settings
 // (on the Options section) so the user can raise the Network Log Size if it's
@@ -399,7 +430,7 @@ const FetchBodyDroppedToastMessage = ({
       out (the log hit its {maxFetchRequests}-request limit), so the body
       couldn't be shown. This usually indicates:
     </Text>
-    <List size="sm" spacing={2}>
+    <ToastCauseList>
       <List.Item>
         a chatty or misbehaving server (notification storms, rapid polling)
       </List.Item>
@@ -410,10 +441,10 @@ const FetchBodyDroppedToastMessage = ({
       <List.Item>
         the Network Log Size set too low for this server&apos;s traffic
       </List.Item>
-    </List>
-    <Anchor component="button" type="button" size="sm" onClick={onAdjust}>
+    </ToastCauseList>
+    <ToastLinkButton onClick={onAdjust}>
       Adjust Network Log Size for this server
-    </Anchor>
+    </ToastLinkButton>
   </Stack>
 );
 
@@ -431,9 +462,9 @@ const OutputValidationToastMessage = ({
       tool&apos;s outputSchema. The inspector renders it anyway, but strict MCP
       clients may not.
     </Text>
-    <Anchor component="button" type="button" size="sm" onClick={onViewDetails}>
+    <ToastLinkButton onClick={onViewDetails}>
       View validation details
-    </Anchor>
+    </ToastLinkButton>
   </Stack>
 );
 
@@ -450,9 +481,9 @@ const UrlElicitationErrorToastMessage = ({
       The server reported a URLElicitationRequired error but listed no required
       elicitations, so there&apos;s nothing to open.
     </Text>
-    <Anchor component="button" type="button" size="sm" onClick={onViewDetails}>
+    <ToastLinkButton onClick={onViewDetails}>
       View error details
-    </Anchor>
+    </ToastLinkButton>
   </Stack>
 );
 
@@ -4183,20 +4214,13 @@ function App() {
     <>
       <Box>
         {reAuthBanner ? (
-          <Box
-            className="reauth-banner-bar"
-            px="md"
-            pt="xs"
-            pos="sticky"
-            top={60}
-            bg="var(--mantine-color-body)"
-          >
+          <ReAuthBannerBar>
             <ReAuthBanner
               message={reAuthBanner.message}
               onReauthenticate={onReauthenticateFromBanner}
               onDismiss={() => setReAuthBanner(null)}
             />
-          </Box>
+          </ReAuthBannerBar>
         ) : null}
         <InspectorView
           deepLink={deepLink}

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

@@ -26,6 +26,12 @@ const colorMap: Record<AnnotationFacet, string> = {
   longRunHint: "yellow",
 };
 
+const FilledBadge = Badge.withProps({
+  variant: "filled",
+  fw: 500,
+  autoContrast: true,
+});
+
 function formatLabel(
   facet: AnnotationFacet,
   value: Role[] | number | boolean,
@@ -59,9 +65,5 @@ export function AnnotationBadge({ facet, value }: AnnotationBadgeProps) {
   // fills and the darker dark-mode `-filled` shades — unlike a fixed
   // scheme→black/white mapping, which inverted the contrast in dark mode.
   // Amber fills are pinned to shade 5 first (see `filledBadgeColor`).
-  return (
-    <Badge color={color} variant="filled" fw={500} autoContrast>
-      {formatLabel(facet, value)}
-    </Badge>
-  );
+  return <FilledBadge color={color}>{formatLabel(facet, value)}</FilledBadge>;
 }

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

@@ -522,6 +522,8 @@ export function AppRenderer({
   // iframe. Sandboxing this outer frame would block the postMessage bridge
   // that `AppBridge` relies on.
   return (
+    // Box+iframe is a native element (not a Mantine primitive), so the
+    // `.withProps()` extraction rule doesn't apply.
     <Box
       component="iframe"
       ref={iframeRef}

clients/web/src/components/elements/ContentViewer/BinaryNotice.tsx

@@ -5,6 +5,8 @@ const ContentWrapper = Flex.withProps({
   direction: "column",
 });
 
+const NoticeCode = Code.withProps({ block: true, p: 36 });
+
 /**
  * Fallback shown when content can't be previewed — an unsupported binary MIME
  * type, or a blob whose base64 fails to decode. Kept in its own module so blob
@@ -15,9 +17,9 @@ export function BinaryNotice({ mimeType }: { mimeType: string }) {
   return (
     <Stack gap="xs">
       <ContentWrapper>
-        <Code block p={36}>
+        <NoticeCode>
           {`[Binary content (${mimeType}) — preview not supported]`}
-        </Code>
+        </NoticeCode>
       </ContentWrapper>
     </Stack>
   );

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

@@ -86,6 +86,11 @@ const PreviewImage = Image.withProps({
   radius: "md",
 });
 
+const CodeBlock = Code.withProps({
+  block: true,
+  p: 36,
+});
+
 // Markdown anchors are constrained to a safe-scheme allowlist: a non-matching
 // href (e.g. `javascript:`, protocol-relative `//evil.com`) renders as inert
 // text so user-supplied markdown can't smuggle a script-bearing link.
@@ -168,9 +173,7 @@ function PlainTextContent({
   const displayText = looksLikeJson(text) ? formatJson(text) : text;
   return (
     <CopyableWrapper copyable={copyable} copyValue={text}>
-      <Code
-        block
-        p={36}
+      <CodeBlock
         // A `block` Code has `overflow: auto`, so make it keyboard-scrollable
         // (WCAG SC 2.1.1) — it holds only text, with no focusable child.
         tabIndex={0}
@@ -180,7 +183,7 @@ function PlainTextContent({
         title={wrap ? undefined : displayText}
       >
         {displayText}
-      </Code>
+      </CodeBlock>
     </CopyableWrapper>
   );
 }
@@ -351,11 +354,11 @@ function BlockContent({
       return (
         <Stack gap="xs">
           <ContentWrapper>
-            <Code block p={36}>
+            <CodeBlock>
               {"text" in block.resource
                 ? block.resource.text
                 : `[blob: ${block.resource.uri}]`}
-            </Code>
+            </CodeBlock>
           </ContentWrapper>
         </Stack>
       );

clients/web/src/components/elements/ContentViewer/CsvTable.tsx

@@ -18,6 +18,13 @@ export const MAX_ROWS = 100;
 
 const PlainCode = Code.withProps({ block: true, variant: "wrapping" });
 
+const CsvGrid = Table.withProps({
+  striped: true,
+  highlightOnHover: true,
+  withTableBorder: true,
+  withColumnBorders: true,
+});
+
 interface ParsedCsv {
   fields: string[];
   rows: string[][];
@@ -49,7 +56,7 @@ export function CsvTable({ text }: CsvTableProps) {
 
   const truncated = parsed.total > MAX_ROWS;
   return (
-    <Table striped highlightOnHover withTableBorder withColumnBorders>
+    <CsvGrid>
       {truncated && (
         <Table.Caption>
           {`Showing first ${MAX_ROWS} of ${parsed.total} rows`}
@@ -71,6 +78,6 @@ export function CsvTable({ text }: CsvTableProps) {
           </Table.Tr>
         ))}
       </Table.Tbody>
-    </Table>
+    </CsvGrid>
   );
 }

clients/web/src/components/elements/ContentViewer/HtmlFrame.tsx

@@ -25,6 +25,8 @@ export function HtmlFrame({ html }: HtmlFrameProps) {
   );
   const url = useObjectUrl(blob);
   return (
+    // Box+iframe is a native element (not a Mantine primitive), so the
+    // `.withProps()` extraction rule doesn't apply.
     <Box
       component="iframe"
       title="HTML preview"

clients/web/src/components/elements/ContentViewer/PdfFrame.tsx

@@ -30,6 +30,8 @@ export function PdfFrame({ data }: PdfFrameProps) {
     return <BinaryNotice mimeType="application/pdf" />;
   }
   return (
+    // Box+iframe is a native element (not a Mantine primitive), so the
+    // `.withProps()` extraction rule doesn't apply.
     <Box
       component="iframe"
       title="PDF preview"

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

@@ -13,21 +13,24 @@ export interface CopyButtonProps {
   flush?: boolean;
 }
 
+const CopyActionIcon = ActionIcon.withProps({
+  variant: "subtle",
+  fz: 24,
+});
+
 export function CopyButton({ value, flush = false }: CopyButtonProps) {
   return (
     <MantineCopyButton value={value}>
       {({ copied, copy }) => (
         <Tooltip label={copied ? "Copied" : "Copy"}>
-          <ActionIcon
-            variant="subtle"
+          <CopyActionIcon
             color={copied ? "green" : "var(--inspector-text-primary)"}
             onClick={copy}
-            fz={24}
             aria-label={copied ? "Copied" : "Copy"}
             {...(flush && { p: 0, h: "auto", w: "auto" })}
           >
             {copied ? "\u2713" : "\u2398"}
-          </ActionIcon>
+          </CopyActionIcon>
         </Tooltip>
       )}
     </MantineCopyButton>