fix(web): correct the Custom Headers OAuth hint, name key/value rows for a11y (#2040)

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

@@ -8,12 +8,15 @@ import {
   type ServerSettingsSection,
 } from "./ServerSettingsForm";
 
-/** Find the "Clear" button living in the rightSection of `input`'s field. */
+/** Find the clear button living in the rightSection of `input`'s field. The
+ *  name is a prefix match because a KeyValueRows field names its clear button
+ *  for the row it belongs to ("Clear header name, Cookie, row 1"), while a standalone
+ *  field keeps the bare "Clear". */
 function clearButtonFor(input: HTMLElement): HTMLElement {
   const root =
     input.closest('[class*="mantine-TextInput-root"]') ??
     input.closest('[class*="Input-wrapper"]');
-  return within(root as HTMLElement).getByRole("button", { name: "Clear" });
+  return within(root as HTMLElement).getByRole("button", { name: /^Clear/ });
 }
 
 const emptySettings: InspectorServerSettings = {
@@ -329,8 +332,11 @@ describe("ServerSettingsForm", () => {
         expandedSections={["headers"]}
       />,
     );
-    const removeButtons = screen.getAllByRole("button", { name: "X" });
-    await user.click(removeButtons[0]);
+    await user.click(
+      screen.getByRole("button", {
+        name: "Remove header, Authorization, row 1",
+      }),
+    );
     expect(onRemoveHeader).toHaveBeenCalledWith(0);
   });
 
@@ -351,8 +357,11 @@ describe("ServerSettingsForm", () => {
     await user.type(valueInput, "Z");
     expect(onMetadataChange).toHaveBeenCalled();
 
-    const removeButtons = screen.getAllByRole("button", { name: "X" });
-    await user.click(removeButtons[0]);
+    await user.click(
+      screen.getByRole("button", {
+        name: "Remove metadata entry, userId, row 1",
+      }),
+    );
     expect(onRemoveMetadata).toHaveBeenCalledWith(0);
   });
 
@@ -676,9 +685,13 @@ describe("ServerSettingsForm", () => {
       await user.type(keyInput, "2");
       expect(onEnvChange).toHaveBeenLastCalledWith(0, "API_KEY2", "secret");
 
-      // The remove ("X") button sits alongside the row's key/value inputs.
-      const removeButtons = screen.getAllByRole("button", { name: "X" });
-      await user.click(removeButtons[removeButtons.length - 1]!);
+      // The remove button sits alongside the row's key/value inputs; its
+      // accessible name identifies which row it belongs to.
+      await user.click(
+        screen.getByRole("button", {
+          name: "Remove environment variable, API_KEY, row 1",
+        }),
+      );
       expect(onRemoveEnv).toHaveBeenCalledWith(0);
     });
   });
@@ -1489,12 +1502,12 @@ describe("ServerSettingsForm", () => {
     expect(
       within(
         keyInput.closest('[class*="Input-wrapper"]') as HTMLElement,
-      ).queryByRole("button", { name: "Clear" }),
+      ).queryByRole("button", { name: /^Clear/ }),
     ).toBeNull();
     expect(
       within(
         valueInput.closest('[class*="Input-wrapper"]') as HTMLElement,
-      ).queryByRole("button", { name: "Clear" }),
+      ).queryByRole("button", { name: /^Clear/ }),
     ).toBeNull();
   });
 
@@ -1517,12 +1530,12 @@ describe("ServerSettingsForm", () => {
     expect(
       within(
         uriInput.closest('[class*="Input-wrapper"]') as HTMLElement,
-      ).queryByRole("button", { name: "Clear" }),
+      ).queryByRole("button", { name: /^Clear/ }),
     ).toBeNull();
     expect(
       within(
         nameInput.closest('[class*="Input-wrapper"]') as HTMLElement,
-      ).queryByRole("button", { name: "Clear" }),
+      ).queryByRole("button", { name: /^Clear/ }),
     ).toBeNull();
     // Typing into the URI threads the empty name through (`root.name ?? ""`).
     await user.type(uriInput, "f");

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

@@ -226,10 +226,22 @@ const ClearStoredOAuthHint = Text.withProps({
 
 function KeyValueRows({
   items,
+  entityLabel,
   onChange,
   onRemove,
 }: {
   items: { key: string; value: string }[];
+  /**
+   * Singular noun for one row ("header", "environment variable", …). Used only
+   * to build each control's `aria-label`: the section heading and the "Key" /
+   * "Value" placeholders are not programmatically associated with the inputs,
+   * so without it an assistive technology cannot tell one section's key box
+   * from another's, and every remove button announces only "X". The row number
+   * rides along because two rows can carry the same key — mid-edit, or a
+   * duplicate a server persisted — and a key-only name would leave both rows'
+   * controls indistinguishable, which is the thing this exists to prevent.
+   */
+  entityLabel: string;
   onChange: (index: number, key: string, value: string) => void;
   onRemove: (index: number) => void;
 }) {
@@ -242,31 +254,50 @@ function KeyValueRows({
 
   return (
     <>
-      {items.map((item, index) => (
-        <Group key={index} grow>
-          <ClearableTextInput
-            placeholder="Key"
-            value={item.key}
-            onChange={(e) => onChange(index, e.currentTarget.value, item.value)}
-            rightSection={
-              item.key ? (
-                <ClearButton onClick={() => onChange(index, "", item.value)} />
-              ) : null
-            }
-          />
-          <ClearableTextInput
-            placeholder="Value"
-            value={item.value}
-            onChange={(e) => onChange(index, item.key, e.currentTarget.value)}
-            rightSection={
-              item.value ? (
-                <ClearButton onClick={() => onChange(index, item.key, "")} />
-              ) : null
-            }
-          />
-          <RemoveIcon onClick={() => onRemove(index)}>X</RemoveIcon>
-        </Group>
-      ))}
+      {items.map((item, index) => {
+        const key = item.key.trim();
+        const rowLabel = key ? `${key}, row ${index + 1}` : `row ${index + 1}`;
+        return (
+          <Group key={index} grow>
+            <ClearableTextInput
+              placeholder="Key"
+              aria-label={`${entityLabel} name, ${rowLabel}`}
+              value={item.key}
+              onChange={(e) =>
+                onChange(index, e.currentTarget.value, item.value)
+              }
+              rightSection={
+                item.key ? (
+                  <ClearButton
+                    aria-label={`Clear ${entityLabel} name, ${rowLabel}`}
+                    onClick={() => onChange(index, "", item.value)}
+                  />
+                ) : null
+              }
+            />
+            <ClearableTextInput
+              placeholder="Value"
+              aria-label={`${entityLabel} value, ${rowLabel}`}
+              value={item.value}
+              onChange={(e) => onChange(index, item.key, e.currentTarget.value)}
+              rightSection={
+                item.value ? (
+                  <ClearButton
+                    aria-label={`Clear ${entityLabel} value, ${rowLabel}`}
+                    onClick={() => onChange(index, item.key, "")}
+                  />
+                ) : null
+              }
+            />
+            <RemoveIcon
+              aria-label={`Remove ${entityLabel}, ${rowLabel}`}
+              onClick={() => onRemove(index)}
+            >
+              X
+            </RemoveIcon>
+          </Group>
+        );
+      })}
     </>
   );
 }
@@ -651,6 +682,7 @@ export function ServerSettingsForm({
               ) : (
                 <KeyValueRows
                   items={settings.env}
+                  entityLabel="environment variable"
                   onChange={onEnvChange}
                   onRemove={onRemoveEnv}
                 />
@@ -666,9 +698,11 @@ export function ServerSettingsForm({
           <Stack gap="md">
             <Group justify="space-between">
               <HintText>
-                Headers sent with every HTTP request to this server. If OAuth is
-                configured below, the `Authorization` header is owned by the
-                OAuth flow and any value set here is ignored.
+                Headers sent with every HTTP request to this server. A custom
+                `Authorization` header takes precedence over an OAuth access
+                token — the SDK transports apply these headers last — so remove
+                it once OAuth is configured, or the flow's token never gets
+                sent.
               </HintText>
               <AddButton onClick={onAddHeader}>+ Add Header</AddButton>
             </Group>
@@ -677,6 +711,7 @@ export function ServerSettingsForm({
             ) : (
               <KeyValueRows
                 items={settings.headers}
+                entityLabel="header"
                 onChange={onHeaderChange}
                 onRemove={onRemoveHeader}
               />
@@ -700,6 +735,7 @@ export function ServerSettingsForm({
             ) : (
               <KeyValueRows
                 items={settings.metadata}
+                entityLabel="metadata entry"
                 onChange={onMetadataChange}
                 onRemove={onRemoveMetadata}
               />

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

@@ -276,8 +276,11 @@ describe("ServerSettingsModal", () => {
       />,
     );
     await user.click(screen.getByRole("button", { name: "Custom Headers" }));
-    const removeButtons = screen.getAllByRole("button", { name: "X" });
-    await user.click(removeButtons[0]);
+    await user.click(
+      screen.getByRole("button", {
+        name: "Remove header, Authorization, row 1",
+      }),
+    );
     expect(onSettingsChange).toHaveBeenCalledWith({
       ...initialSettings,
       headers: [],
@@ -342,10 +345,13 @@ describe("ServerSettingsModal", () => {
       />,
     );
     await user.click(screen.getByRole("button", { name: "Request Metadata" }));
-    const removeButtons = screen.getAllByRole("button", { name: "X" });
-    // After expanding metadata, both header and metadata X buttons exist;
-    // the metadata X is the last one.
-    await user.click(removeButtons[removeButtons.length - 1]);
+    // Both the header and metadata rows have a remove button; each is named
+    // for the row it belongs to, so no positional guess is needed.
+    await user.click(
+      screen.getByRole("button", {
+        name: "Remove metadata entry, userId, row 1",
+      }),
+    );
     expect(onSettingsChange).toHaveBeenCalledWith({
       ...initialSettings,
       metadata: [],
@@ -810,8 +816,11 @@ describe("ServerSettingsModal", () => {
       await user.click(
         screen.getByRole("button", { name: "Environment Variables" }),
       );
-      const removeButtons = screen.getAllByRole("button", { name: "X" });
-      await user.click(removeButtons[removeButtons.length - 1]);
+      await user.click(
+        screen.getByRole("button", {
+          name: "Remove environment variable, A, row 1",
+        }),
+      );
       expect(onSettingsChange).toHaveBeenCalledWith({
         ...emptySettings,
         env: [],