fix: address Copilot review round 37 — stale constants, inherited answers, prefixed labels

- The seeding effect re-applies constants as well as filling absences: an
  in-place schema change can move a `const` the user cannot edit, and the
  read-only field then displayed the new value while `values` still held the
  old one — which is what would be submitted. Ordinary defaults are still only
  ever added, so an edited field keeps what the user put there.
- hasMissingIn checks own-property presence: a required argument legally named
  `constructor` resolved to the inherited one and read as supplied, enabling
  a submit the schema rejects.
- The TUI keeps the declared name as a branch field's fallback title. The
  prefix is an internal field NAME, and buildFields labels from its map key —
  so the showcase displayed `__b0__address` where the schema says
  `address`.

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

Author: cliffhall

clients/tui/__tests__/schemaToForm.test.ts

@@ -465,6 +465,35 @@ describe("schemaToForm", () => {
       ]);
     });
 
+    it("labels a branch field by the name the schema declared", () => {
+      // The prefix is an internal field name; `buildFields` falls back to its
+      // map key for a label, so the user would otherwise see `__b0__address`.
+      const form = schemaToForm(UNION, "union_tool");
+      expect(form.sections[1]!.fields[1]).toMatchObject({
+        name: "__b0__address",
+        label: "address",
+      });
+    });
+
+    it("keeps a declared title ahead of the fallback", () => {
+      const form = schemaToForm(
+        {
+          type: "object",
+          anyOf: [
+            {
+              type: "object",
+              properties: { a: { type: "string", title: "Street address" } },
+            },
+            { type: "object", properties: { b: { type: "string" } } },
+          ],
+        },
+        "titled",
+      );
+      expect(form.sections[1]!.fields[0]).toMatchObject({
+        label: "Street address",
+      });
+    });
+
     it("offers a variant select listing the alternatives", () => {
       const form = schemaToForm(UNION, "union_tool");
       expect(form.sections[0]!.fields[0]).toMatchObject({

clients/tui/src/utils/schemaToForm.ts

@@ -143,6 +143,19 @@ function branchFields(
   return [...new Set([...own, ...sharedFieldNames(base, branches)])];
 }
 
+/**
+ * A property declaration carrying the name it was declared under as its
+ * fallback `title`, so a renamed field still displays the schema's own name.
+ */
+function labelled(property: unknown, name: string): unknown {
+  if (typeof property !== "object" || property === null) {
+    // JSON Schema's `true` constrains nothing, so a title-only object says the
+    // same thing and carries the label.
+    return { title: name };
+  }
+  return { title: name, ...property };
+}
+
 /** The `const` a property schema pins its value to, if any. */
 function constOf(schema: unknown): unknown {
   if (typeof schema !== "object" || schema === null) return undefined;
@@ -217,7 +230,11 @@ export function schemaToForm(
     const properties = Object.fromEntries(
       branchFields(base, branches, index).map((name) => [
         branchFieldName(prefix, index, name),
-        branch.schema.properties?.[name],
+        // The prefix is an internal field NAME, never a label: `buildFields`
+        // falls back to its map key when a declaration carries no `title`, so
+        // without this the form would show `__b0__address` where the schema
+        // says `address`.
+        labelled(branch.schema.properties?.[name], name),
       ]),
     );
     sections.push({

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

@@ -2391,6 +2391,43 @@ describe("SchemaForm multiline strings (#2042)", () => {
       expect(screen.getByRole("textbox", { name: /Address/ })).toBeTruthy();
     });
 
+    it("corrects a stale const when the schema changes in place", async () => {
+      // The user cannot edit a read-only field, so a `const` that moves under
+      // an unchanged `resetKey` would display the new value while the old one
+      // sat in `values`, waiting to be submitted.
+      const onChange = vi.fn();
+      const pinned = (value: string): InspectorFormSchema => ({
+        type: "object",
+        properties: {
+          kind: { type: "string", const: value },
+          note: { type: "string", title: "Note" },
+        },
+      });
+      const { rerender } = renderWithMantine(
+        <SchemaForm
+          schema={pinned("email")}
+          values={{ kind: "email", note: "kept" }}
+          onChange={onChange}
+          resetKey="same-tool"
+        />,
+      );
+      await Promise.resolve();
+      onChange.mockClear();
+
+      rerender(
+        <SchemaForm
+          schema={pinned("sms")}
+          values={{ kind: "email", note: "kept" }}
+          onChange={onChange}
+          resetKey="same-tool"
+        />,
+      );
+      // The constant is corrected; what the user typed is left alone.
+      await waitFor(() =>
+        expect(onChange).toHaveBeenCalledWith({ kind: "sms", note: "kept" }),
+      );
+    });
+
     it("renders no picker for a single-branch union but still shows its fields", () => {
       const schema: InspectorFormSchema = {
         type: "object",

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

@@ -36,7 +36,10 @@ import {
   resolveRootUnion,
   selectBranchIndex,
 } from "@inspector/core/json/rootUnion.js";
-import { collectSchemaDefaults } from "../../../utils/jsonUtils";
+import {
+  applySchemaConstants,
+  collectSchemaDefaults,
+} from "../../../utils/jsonUtils";
 
 const FieldLabel = Text.withProps({
   fw: 500,
@@ -751,9 +754,20 @@ export function SchemaForm({
     const missing = Object.entries(collectSchemaDefaults(current)).filter(
       ([name]) => !Object.hasOwn(held, name),
     );
-    if (missing.length > 0) {
-      report({ ...held, ...Object.fromEntries(missing) });
-    }
+    // Constants are re-applied as well as seeded: an in-place schema change can
+    // move a `const` the user cannot edit — the read-only field then displays
+    // the new value while `values` still holds the old one, which is what would
+    // be submitted. Ordinary defaults are only ever *added*, so an edited field
+    // keeps what the user put there.
+    const next = applySchemaConstants(
+      current,
+      missing.length > 0 ? { ...held, ...Object.fromEntries(missing) } : held,
+    );
+    const changed = Object.keys(next).some(
+      (name) =>
+        !Object.hasOwn(held, name) || !Object.is(next[name], held[name]),
+    );
+    if (changed) report(next);
     // Keyed by the entity and branch being edited, and by the fixed values
     // themselves: a tool refreshed in place keeps its `resetKey` and its branch
     // while its schema changes underneath, and a newly pinned field would

clients/web/src/utils/jsonUtils.test.ts

@@ -601,6 +601,20 @@ describe("root composition (#2123)", () => {
     ).toBe(true);
   });
 
+  it("does not read an inherited property as a supplied answer", () => {
+    // `constructor` is a legal argument name; reading it off the prototype
+    // would enable a submit the schema rejects.
+    const schema: InspectorFormSchema = {
+      type: "object",
+      // Built through `fromEntries`: in an object literal, `constructor` is
+      // TypeScript's own inherited member rather than a plain key.
+      properties: Object.fromEntries([["constructor", { type: "string" }]]),
+      required: ["constructor"],
+    };
+    expect(hasMissingRequiredFields(schema, {})).toBe(true);
+    expect(hasMissingRequiredFields(schema, { constructor: "x" })).toBe(false);
+  });
+
   it("blocks submission while no branch is satisfied", () => {
     expect(hasMissingRequiredFields(UNION, {})).toBe(true);
     expect(hasMissingRequiredFields(UNION, { kind: "email" })).toBe(true);

clients/web/src/utils/jsonUtils.ts

@@ -352,6 +352,10 @@ function hasMissingIn(
   const required = Array.isArray(schema.required) ? schema.required : [];
   const properties = schema.properties ?? {};
   return required.some((field) => {
+    // `hasOwn` first: an argument legally named `constructor` would otherwise
+    // resolve to the inherited one and read as supplied, enabling a submit the
+    // schema rejects.
+    if (!Object.hasOwn(values, field)) return true;
     const value = values[field];
     if (value === null) {
       const fieldSchema = properties[field];