Merge branch 'v2/main' into v2/fix/2207-tui-force-color-ansi

clients/tui/__tests__/schemaToForm.test.ts

@@ -656,6 +656,58 @@ describe("schemaToForm", () => {
       expect(names).toContain("__b_0____b0__x");
     });
 
+    // #2224: `required` lists names, not declarations, so a member may require
+    // one it never declares. `buildFields` enumerates `properties` alone, so no
+    // control was rendered for it while `missingRequiredFields` reported it
+    // missing at every submit — a section the user could never complete.
+    it("declines a member requiring a name it never declares", () => {
+      const schema = {
+        type: "object",
+        anyOf: [
+          {
+            type: "object",
+            properties: { kind: { type: "string", const: "a" } },
+            required: ["kind", "payload"],
+          },
+          {
+            type: "object",
+            properties: { kind: { type: "string", const: "b" } },
+            required: ["kind"],
+          },
+        ],
+      };
+      const form = schemaToForm(schema, "undeclared_required");
+      // No variant select and no per-branch section: the union is declined, so
+      // the root's own (here empty) properties are what render.
+      expect(form.sections).toEqual([{ title: "Parameters", fields: [] }]);
+      // And nothing is reported missing, so the call is no longer blocked on a
+      // field that has nowhere to be typed.
+      expect(
+        missingRequiredFields(schema, decodeFormValues(schema, {})),
+      ).toEqual([]);
+    });
+
+    it("keeps offering a member requiring a name the root declares", () => {
+      // The regression guard for the check above: the merge is what is judged,
+      // and `anyOf: [{ required: ["email"] }, …]` is an ordinary union.
+      const form = schemaToForm(
+        {
+          type: "object",
+          properties: {
+            email: { type: "string" },
+            phone: { type: "string" },
+          },
+          anyOf: [
+            { type: "object", required: ["email"] },
+            { type: "object", required: ["phone"] },
+          ],
+        },
+        "inherited_required",
+      );
+      expect(form.sections).toHaveLength(3);
+      expect(form.sections[0]!.fields[0]).toMatchObject({ name: "__variant" });
+    });
+
     describe("decodeFormValues", () => {
       it("submits the chosen branch's fields under their real names", () => {
         expect(

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

@@ -2284,6 +2284,86 @@ describe("SchemaForm raw JSON (#2151)", () => {
     await enableRawJson(user);
     expect(screen.getAllByLabelText("Edit as JSON")).toHaveLength(1);
   });
+
+  // #2224: a root union whose alternatives are all declined resolves to a base
+  // with nothing on it, so the form renders no picker and no fields. The switch
+  // is the only way to make the call — open it rather than leaving the user in
+  // front of a blank form to work that out.
+  describe("fallback for an unrenderable root union (#2224)", () => {
+    // Every member requires a name it never declares, so none is offerable.
+    const deadEnd: InspectorFormSchema = {
+      type: "object",
+      anyOf: [
+        {
+          type: "object",
+          properties: { kind: { type: "string", const: "a" } },
+          required: ["kind", "payload"],
+        },
+        {
+          type: "object",
+          properties: { kind: { type: "string", const: "b" } },
+          required: ["kind", "payload"],
+        },
+      ],
+    };
+
+    function switchElement(): HTMLInputElement {
+      return screen.getByLabelText("Edit as JSON") as HTMLInputElement;
+    }
+
+    it("opens the editor when the form would otherwise be empty", () => {
+      renderWithMantine(<RawHarness schema={deadEnd} />);
+      expect(switchElement().checked).toBe(true);
+      expect(getAceTextByLabel(/Arguments JSON/)).toBe("{}");
+    });
+
+    it("leaves the editor closed for a tool that takes no arguments", () => {
+      // Nothing to render here either, but nothing is missing: seeding a JSON
+      // editor for `{}` would be noise on every no-argument tool.
+      renderWithMantine(
+        <RawHarness schema={{ type: "object", properties: {} }} />,
+      );
+      expect(switchElement().checked).toBe(false);
+    });
+
+    it("still lets the user switch back to the fields", async () => {
+      const user = userEvent.setup();
+      renderWithMantine(<RawHarness schema={deadEnd} />);
+      await user.click(switchElement());
+      expect(switchElement().checked).toBe(false);
+    });
+
+    it("opens the editor for a schema that arrives after the first render", () => {
+      // The form is reused across tools rather than remounted, so a `useState`
+      // initializer alone would only ever see the tool it mounted on.
+      const { rerender } = renderWithMantine(
+        <SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
+      );
+      expect(switchElement().checked).toBe(false);
+
+      rerender(<SchemaForm schema={deadEnd} values={{}} onChange={vi.fn()} />);
+      expect(switchElement().checked).toBe(true);
+    });
+
+    it("does not close an editor the user opened when the schema changes", async () => {
+      // One-way: the fallback opens the editor, and nothing closes it but the
+      // user.
+      const user = userEvent.setup();
+      const { rerender } = renderWithMantine(
+        <SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
+      );
+      await enableRawJson(user);
+
+      rerender(
+        <SchemaForm
+          schema={{ type: "object", properties: {} }}
+          values={{}}
+          onChange={vi.fn()}
+        />,
+      );
+      expect(switchElement().checked).toBe(true);
+    });
+  });
 });
 
 // A plain `<input>` swallows Enter, so a string argument could not be given a

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

@@ -931,7 +931,26 @@ export function SchemaForm({
   // rather than reported under a reserved name: field names come straight out
   // of a server's schema, so any sentinel this form invented could collide with
   // a real argument and clear a block the user cannot see.
-  const [rawJsonMode, setRawJsonMode] = useState(false);
+  //
+  // A root union whose alternatives are all declined resolves to a base with
+  // nothing on it, so the form renders neither a picker nor a field — the user
+  // is left staring at a switch they have to know to reach for in order to make
+  // the call at all (#2224). Open the editor for them in that one case: the
+  // arguments are still expressible, just not as fields. Seeded rather than
+  // forced, so the switch keeps working; re-seeded during render (never in an
+  // effect) because this form is reused across tools rather than remounted, and
+  // a `useState` initializer would only ever see the first one.
+  const rawJsonFallback =
+    allowRawJson &&
+    branches.length === 0 &&
+    (schema.oneOf !== undefined || schema.anyOf !== undefined) &&
+    Object.keys(properties).length === 0;
+  const [rawJsonMode, setRawJsonMode] = useState(rawJsonFallback);
+  useValueChange(rawJsonFallback, (fallback) => {
+    // One-way: a schema that stops needing the fallback does not close an
+    // editor the user may have opened on purpose.
+    if (fallback) setRawJsonMode(true);
+  });
   const [rawJsonInvalid, setRawJsonInvalid] = useState(false);
 
   // Stable so `RawArgumentsField`'s reporting effect subscribes once rather

clients/web/src/test/core/rootUnion.test.ts

@@ -809,6 +809,84 @@ describe("resolveRootUnion", () => {
       expect(branches).toHaveLength(2);
     });
 
+    it("declines a member requiring a name nothing declares", () => {
+      // `required` is a list of names, not of declarations. Both form builders
+      // enumerate `properties` alone, so `payload` gets no control while the
+      // submit gate reports it missing forever — the picker offers an option
+      // that can never be completed (#2224).
+      const { branches } = resolveRootUnion({
+        type: "object",
+        anyOf: [
+          {
+            type: "object",
+            properties: { kind: { const: "a" } },
+            required: ["kind", "payload"],
+          },
+          SMS,
+        ],
+      });
+      expect(branches).toEqual([]);
+    });
+
+    it("declines a member requiring a name only ANOTHER member declares", () => {
+      // Branches are alternatives, not a conjunction: a name the sibling
+      // declares is not one this branch can render, so the dead end is the
+      // same one.
+      const { branches } = resolveRootUnion({
+        type: "object",
+        anyOf: [
+          {
+            type: "object",
+            properties: { kind: { const: "a" } },
+            required: ["kind", "phone"],
+          },
+          SMS,
+        ],
+      });
+      expect(branches).toEqual([]);
+    });
+
+    it("offers a member requiring a name it inherits from the root", () => {
+      // The merge is what is judged, so a member requiring a name the ROOT
+      // declares stays offerable — the `anyOf: [{ required: ["email"] }, …]`
+      // shape above must not regress.
+      const { branches } = resolveRootUnion({
+        type: "object",
+        properties: { token: { type: "string" } },
+        anyOf: [
+          {
+            type: "object",
+            properties: { a: { type: "string" } },
+            required: ["token"],
+          },
+          {
+            type: "object",
+            properties: { b: { type: "string" } },
+            required: ["token"],
+          },
+        ],
+      });
+      expect(branches).toHaveLength(2);
+    });
+
+    it("does not read an inherited name as a declared property", () => {
+      // An argument legally named `constructor` resolves through the prototype
+      // on a plain object, so a membership test that is not `hasOwn` would call
+      // this branch offerable and render nothing for it.
+      const { branches } = resolveRootUnion({
+        type: "object",
+        anyOf: [
+          {
+            type: "object",
+            properties: { kind: { const: "a" } },
+            required: ["constructor"],
+          },
+          SMS,
+        ],
+      });
+      expect(branches).toEqual([]);
+    });
+
     it("declines a member carrying a `false` property schema", () => {
       // `false` admits no value at all, so the field can never be filled — and
       // a required one makes the branch unsatisfiable.
@@ -919,6 +997,24 @@ describe("resolveRootUnion", () => {
       ).toBe(true);
     });
 
+    it("still counts a union whose every branch is now declined", () => {
+      // The #2224 check makes such a union render nothing, which must not be
+      // read as "this tool takes no arguments" — an App tool carrying one would
+      // then be auto-invoked with `{}` rather than asked about.
+      expect(
+        declaresAnyFields({
+          type: "object",
+          oneOf: [
+            {
+              type: "object",
+              properties: { kind: { const: "a" } },
+              required: ["kind", "payload"],
+            },
+          ],
+        }),
+      ).toBe(true);
+    });
+
     it("counts a required name a schema never declares", () => {
       // Legal, and the tool plainly takes an argument — an App tool shaped this
       // way must ask rather than being auto-invoked with `{}`.