fix: dedupe a repeated name in a group, and name every unmet requirement

Addresses Copilot's round-13 review on #2035. Two suppressed comments, both
real, both in the required-group work from round 7 rather than in the recent
preview rework.

1. `{a,a}` is ONE requirement named twice, but `requiredGroups` stored it as
   a two-name group, so everything downstream read it as "either of these
   will do". The TUI was left with a form it could not submit: its
   `length === 1` test marked `a` optional while ResourceTestModal's guard
   still refused a blank. The web panel offered the useless hint
   "Any one of: a, a". Names are now deduplicated within the expression --
   expansion still emits both occurrences (`x://{a,a}` -> `x://1,1`), which
   RFC 6570 requires.

2. The per-field hint used `find`, so a variable in several shared groups
   advertised only the first. With `{a,b}{b,c}{a,c}`, filling `b` satisfies
   the first two and every visible hint then looks met while Read Resource
   stays disabled on the unmet `{a,c}` -- a requirement with nothing on
   screen pointing at it.

   Fields now hint every shared group they sit in. That alone is not enough,
   because no per-field hint can say WHICH group is still outstanding, so
   the form states it directly: "Still needed: a or c", built from the same
   `unmetRequiredGroups` the submit gate uses, so the message and the button
   cannot disagree. `hasRequiredValues` is no longer called here -- the
   panel derives both from the one list.

npm run ci passes.

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

clients/tui/__tests__/uriTemplateToForm.test.ts

@@ -64,3 +64,15 @@ describe("uriTemplateToForm", () => {
     expect(form.sections[0]!.fields).toEqual([]);
   });
 });
+
+describe("a name repeated inside one expression", () => {
+  it("is required, not treated as a shared group", () => {
+    // `{a,a}` is one requirement named twice. Before core deduplicated the
+    // group, `requiredGroups` returned ["a","a"], so this form's
+    // `length === 1` test left the field optional while ResourceTestModal's
+    // submit guard still refused a blank -- an un-submittable form.
+    const [field] = uriTemplateToForm("x://{a,a}", "T").sections[0].fields;
+    expect(field.name).toBe("a");
+    expect(field.required).toBe(true);
+  });
+});

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

@@ -266,6 +266,58 @@ describe("ResourceTemplatePanel", () => {
       expect(onReadResource).not.toHaveBeenCalled();
     });
 
+    it("names every unmet group, so a disabled button always has a reason", async () => {
+      // With `{a,b}{b,c}{a,c}`, filling `b` satisfies the first two groups. A
+      // per-field hint then looks satisfied while Read Resource stays disabled
+      // on the third -- so the outstanding requirement is stated form-level.
+      const user = userEvent.setup();
+      renderWithMantine(
+        <ResourceTemplatePanel
+          template={{ name: "Shared", uriTemplate: "x://{a,b}{b,c}{a,c}" }}
+          onReadResource={vi.fn()}
+        />,
+      );
+      expect(screen.getByText(/Still needed:/)).toHaveTextContent(
+        "Still needed: a or b; b or c; a or c",
+      );
+      await user.type(screen.getByLabelText("b"), "1");
+      expect(screen.getByText(/Still needed:/)).toHaveTextContent(
+        "Still needed: a or c",
+      );
+      expect(
+        screen.getByRole("button", { name: "Read Resource" }),
+      ).toBeDisabled();
+      await user.type(screen.getByLabelText("a"), "2");
+      expect(screen.queryByText(/Still needed:/)).not.toBeInTheDocument();
+      expect(
+        screen.getByRole("button", { name: "Read Resource" }),
+      ).toBeEnabled();
+    });
+
+    it("hints every shared group a variable sits in, not just the first", () => {
+      renderWithMantine(
+        <ResourceTemplatePanel
+          template={{ name: "Shared", uriTemplate: "x://{a,b}{a,c}" }}
+          onReadResource={vi.fn()}
+        />,
+      );
+      expect(
+        screen.getByText("Any one of each: a, b; a, c"),
+      ).toBeInTheDocument();
+    });
+
+    it("treats a name repeated in one expression as a plain requirement", () => {
+      renderWithMantine(
+        <ResourceTemplatePanel
+          template={{ name: "Twice", uriTemplate: "x://{a,a}" }}
+          onReadResource={vi.fn()}
+        />,
+      );
+      // Not "Any one of: a, a" -- it is one field, required.
+      expect(screen.queryByText(/Any one of/)).not.toBeInTheDocument();
+      expect(screen.getByText("Still needed: a")).toBeInTheDocument();
+    });
+
     it("shows the malformed template unexpanded in the preview", () => {
       renderWithMantine(
         <ResourceTemplatePanel

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

@@ -16,11 +16,11 @@ import { AnnotationBadge } from "../../elements/AnnotationBadge/AnnotationBadge"
 import { CopyButton } from "../../elements/CopyButton/CopyButton";
 import {
   definedValues,
-  hasRequiredValues,
   previewUriTemplate,
   requiredGroups,
   templateVariables,
   tryExpandUriTemplate,
+  unmetRequiredGroups,
 } from "../../../utils/uriTemplate";
 
 export interface ResourceTemplatePanelProps {
@@ -77,6 +77,13 @@ const DescriptionText = Text.withProps({
 // value that cannot be encoded -- so a screen-reader user would otherwise meet
 // a silently disabled action. An alert is an assertive live region, which is
 // what announces text that arrives after first render.
+// What is still missing before Read Resource can fire. Dimmed rather than red:
+// an incomplete form is the expected starting state, not an error.
+const RequirementText = Text.withProps({
+  size: "sm",
+  c: "dimmed",
+});
+
 const ExpansionErrorText = Text.withProps({
   size: "sm",
   c: "red",
@@ -255,8 +262,11 @@ export function ResourceTemplatePanel({
   // untouched field is indistinguishable from a deliberately empty one. That is
   // a fact about the form, so the form is what resolves it.
   const expansion = tryExpandUriTemplate(uriTemplate, definedValues(variables));
-  const canSubmit =
+  // Named rather than merely counted, so a disabled Read Resource always has a
-    expansion.error === undefined && hasRequiredValues(groups, variables);
+  // reason on screen. Per-field hints cannot carry this: a variable shared
+  // across several groups looks satisfied once any one of them is met.
+  const unmet = unmetRequiredGroups(groups, variables);
+  const canSubmit = expansion.error === undefined && unmet.length === 0;
 
   function handleSubmit() {
     /* v8 ignore next -- unreachable: the button is disabled whenever the
@@ -293,15 +303,23 @@ export function ResourceTemplatePanel({
           const individuallyRequired = groups.some(
             (names) => names.length === 1 && names[0] === varName,
           );
-          const sharedGroup = individuallyRequired
+          // EVERY shared group this variable sits in, not the first. With
-            ? undefined
+          // `{a,b}{b,c}{a,c}`, showing only the first left `b` looking like it
-            : groups.find(
+          // satisfied everything while Read Resource stayed disabled on the
+          // unmet `{a,c}` -- a hidden requirement is worse than a wordy hint.
+          // The form-level "Still needed" line below names what is actually
+          // outstanding, which is the part a per-field hint cannot express.
+          const sharedGroups = individuallyRequired
+            ? []
+            : groups.filter(
                 (names) => names.length > 1 && names.includes(varName),
               );
           const description = !required
             ? "Optional"
-            : sharedGroup
+            : sharedGroups.length > 0
-              ? `Any one of: ${sharedGroup.join(", ")}`
+              ? `Any one of${sharedGroups.length > 1 ? " each" : ""}: ${sharedGroups
+                  .map((names) => names.join(", "))
+                  .join("; ")}`
               : undefined;
           return useAutocomplete ? (
             <Autocomplete
@@ -344,6 +362,11 @@ export function ResourceTemplatePanel({
       {expansion.error !== undefined && (
         <ExpansionErrorText>{expansion.error}</ExpansionErrorText>
       )}
+      {expansion.error === undefined && unmet.length > 0 && (
+        <RequirementText>
+          Still needed: {unmet.map((names) => names.join(" or ")).join("; ")}
+        </RequirementText>
+      )}
       <FooterRow>
         <Button size="sm" disabled={!canSubmit} onClick={handleSubmit}>
           Read Resource

clients/web/src/test/core/mcp/uriTemplate.test.ts

@@ -352,6 +352,24 @@ describe("requiredGroups / hasRequiredValues", () => {
   });
 });
 
+describe("a name repeated inside one expression", () => {
+  // `{a,a}` is one requirement named twice, not two names either of which
+  // would do. Undeduplicated it read as a shared group everywhere downstream:
+  // the TUI's `length === 1` test marked `a` optional while its submit guard
+  // still refused a blank, and the web panel offered "Any one of: a, a".
+  it("collapses to a single-name required group", () => {
+    expect(requiredGroups("x://{a,a}")).toEqual([["a"]]);
+  });
+
+  it("still expands both occurrences, which RFC 6570 requires", () => {
+    expect(expandUriTemplate("x://{a,a}", { a: "1" })).toBe("x://1,1");
+  });
+
+  it("leaves genuinely distinct names alone", () => {
+    expect(requiredGroups("x://{a,b}")).toEqual([["a", "b"]]);
+  });
+});
+
 describe("expression independence", () => {
   it("does not rewrite a later query expression on the own-expansion path", () => {
     expect(

clients/web/src/utils/uriTemplate.ts

@@ -26,6 +26,7 @@ export {
   requiredGroups,
   templateVariables,
   tryExpandUriTemplate,
+  unmetRequiredGroups,
 } from "@inspector/core/mcp/uriTemplate.js";
 export type {
   TemplateExpansion,

core/mcp/uriTemplate.ts

@@ -317,7 +317,12 @@ export function requiredGroups(uriTemplate: string): string[][] {
     // time on a condition nothing can satisfy, and the "any one of" message
     // built from it would name no fields.
     if (part.names.length === 0) continue;
-    groups.push(part.names);
+    // Deduplicated within the expression: `{a,a}` is one requirement named
+    // twice, not two names either of which would do. Left as-is it read as a
+    // shared group everywhere downstream -- the TUI's `length === 1` test
+    // marked `a` optional while its submit guard still refused a blank, and the
+    // web panel offered the useless hint "Any one of: a, a".
+    groups.push([...new Set(part.names)]);
   }
   return groups;
 }