fix: accessible names for the authorization-parameter rows (#2018)

Copilot round 2 (suppressed comments). Five of the seven findings were the
accessibility of the new row list and one stray cast; all are addressed here.

- The reserved-key reason is now the key input's `error` **string** rather than
  a bare boolean plus free-standing text. Mantine renders it in the input's own
  error slot and wires the `aria-describedby` association, so a screen reader
  learns *why* the field is invalid; a boolean only set `aria-invalid`. This
  also deletes the duplicate `ReservedParamError` text and its Stack wrapper.
- Each control carries a per-row `aria-label` — the section heading and the
  placeholders are not programmatically associated with the inputs, so the key
  box, the value box, and a remove button announcing only "X" were
  indistinguishable to assistive technology. The label names the row by its key,
  falling back to "row N" while the key is still blank.
- Dropped an unnecessary `as unknown as Record<string, unknown>` in the
  servers-route integration test: `MCPConfig.mcpServers` is already typed as
  `Record<string, StoredMCPServer>`, so the entry's `oauth` reads directly.

Three tests cover the new behavior (accessible names, the blank-key row-number
fallback, and the aria-describedby association), each verified to fail against
the pre-fix component. No change to the reserved-key rule or the EMA boundary.

Signed-off-by: cliffhall <cliff@futurescale.com>

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

@@ -826,6 +826,83 @@ describe("ServerSettingsForm", () => {
     );
   });
 
+  // #2018 — the section heading and the placeholders are not programmatically
+  // associated with these inputs, so each control carries its own aria-label and
+  // the reserved-key reason is the input's `error` string (which Mantine wires
+  // to the input via aria-describedby) rather than free-standing text.
+  it("gives each authorization-parameter control an accessible name", () => {
+    renderWithMantine(
+      <ServerSettingsForm
+        {...baseHandlers}
+        settings={{
+          ...emptySettings,
+          oauthAuthorizationParams: [{ key: "kc_idp_hint", value: "corp" }],
+        }}
+        expandedSections={["oauth"]}
+      />,
+    );
+    expect(
+      screen.getByRole("textbox", {
+        name: "Authorization parameter name, kc_idp_hint",
+      }),
+    ).toHaveValue("kc_idp_hint");
+    expect(
+      screen.getByRole("textbox", {
+        name: "Authorization parameter value, kc_idp_hint",
+      }),
+    ).toHaveValue("corp");
+    expect(
+      screen.getByRole("button", {
+        name: "Remove authorization parameter kc_idp_hint",
+      }),
+    ).toBeInTheDocument();
+  });
+
+  it("falls back to a row number in the accessible name when the key is blank", () => {
+    renderWithMantine(
+      <ServerSettingsForm
+        {...baseHandlers}
+        settings={{
+          ...emptySettings,
+          oauthAuthorizationParams: [{ key: "", value: "" }],
+        }}
+        expandedSections={["oauth"]}
+      />,
+    );
+    expect(
+      screen.getByRole("textbox", {
+        name: "Authorization parameter name, row 1",
+      }),
+    ).toBeInTheDocument();
+  });
+
+  // The reason must reach assistive tech through the input, not just as text on
+  // the page: Mantine emits the `error` string with an id the input references.
+  it("associates the reserved-key reason with the key input", () => {
+    renderWithMantine(
+      <ServerSettingsForm
+        {...baseHandlers}
+        settings={{
+          ...emptySettings,
+          oauthAuthorizationParams: [{ key: "state", value: "spoofed" }],
+        }}
+        expandedSections={["oauth"]}
+      />,
+    );
+    const keyInput = screen.getByRole("textbox", {
+      name: "Authorization parameter name, state",
+    });
+    const describedBy = keyInput.getAttribute("aria-describedby");
+    expect(describedBy).toBeTruthy();
+    const reason = describedBy
+      ?.split(" ")
+      .map((id) => document.getElementById(id)?.textContent ?? "")
+      .join(" ");
+    expect(reason).toContain(
+      '"state" is set by the authorization flow and cannot be overridden.',
+    );
+  });
+
   it("does not flag a blank authorization-parameter row as reserved", () => {
     renderWithMantine(
       <ServerSettingsForm

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

@@ -202,11 +202,6 @@ const FieldDescription = Text.withProps({
   c: "dimmed",
 });
 
-const ReservedParamError = Text.withProps({
-  size: "xs",
-  c: "var(--inspector-danger-text)",
-});
-
 const ClearStoredOAuthButton = Button.withProps({
   variant: "light",
   color: "red",
@@ -268,9 +263,17 @@ function KeyValueRows({
   );
 }
 
-// Reserved-key rejection is inline per row (#2018): the key input goes red with
-// the reason under it, and the section-level warning below repeats it so the
-// rejection is visible even when the offending row is scrolled out of view.
+// Reserved-key rejection is inline per row (#2018): the reason is passed as the
+// key input's `error` **string**, so Mantine renders it in the input's own error
+// slot and wires the `aria-describedby` association — a bare boolean would mark
+// the field invalid without telling a screen reader why. The section-level Alert
+// below repeats it so the rejection stays visible when the offending row is
+// scrolled out of view.
+//
+// Each control carries a per-row `aria-label`: the section heading and the
+// placeholders are not programmatically associated with the inputs, so without
+// one an assistive technology cannot tell the key box from the value box, and
+// the remove button announces only "X".
 function AuthorizationParamRows({
   params,
   onChange,
@@ -282,13 +285,15 @@ function AuthorizationParamRows({
 }) {
   return (
     <>
-      {params.map((param, index) => (
-        <Stack key={index} gap={4}>
-          <Group grow>
+      {params.map((param, index) => {
+        const rowLabel = param.key.trim() || `row ${index + 1}`;
+        return (
+          <Group key={index} grow align="flex-start">
             <ClearableTextInput
+              aria-label={`Authorization parameter name, ${rowLabel}`}
               placeholder="Parameter (e.g. kc_idp_hint)"
               value={param.key}
-              error={authorizationParamKeyError(param.key) !== undefined}
+              error={authorizationParamKeyError(param.key)}
               onChange={(e) =>
                 onChange(index, e.currentTarget.value, param.value)
               }
@@ -301,6 +306,7 @@ function AuthorizationParamRows({
               }
             />
             <ClearableTextInput
+              aria-label={`Authorization parameter value, ${rowLabel}`}
               placeholder="Value"
               value={param.value}
               onChange={(e) =>
@@ -312,15 +318,15 @@ function AuthorizationParamRows({
                 ) : null
               }
             />
-            <RemoveIcon onClick={() => onRemove(index)}>X</RemoveIcon>
+            <RemoveIcon
+              aria-label={`Remove authorization parameter ${rowLabel}`}
+              onClick={() => onRemove(index)}
+            >
+              X
+            </RemoveIcon>
           </Group>
-          {authorizationParamKeyError(param.key) ? (
-            <ReservedParamError>
-              {authorizationParamKeyError(param.key)}
-            </ReservedParamError>
-          ) : null}
-        </Stack>
-      ))}
+        );
+      })}
     </>
   );
 }

clients/web/src/test/integration/mcp/remote/servers-route.test.ts

@@ -471,9 +471,8 @@ describe("/api/servers routes", () => {
         }),
       });
       expect(res.status).toBe(200);
-      const stored = readConfig(h.configPath).mcpServers
-        .authparams as unknown as Record<string, unknown>;
-      expect(stored.oauth).toEqual({
+      const stored = readConfig(h.configPath).mcpServers.authparams;
+      expect(stored?.oauth).toEqual({
         authorizationParams: { kc_idp_hint: "corp" },
       });
     });