fix(mcp): one issue per broken object, and a missing rotation still opens (#457)

Author: u8array

packages/core/src/lib/designFile.ts

@@ -32,7 +32,7 @@ export type DesignFileError = "parse_error" | "invalid_schema" | "fn_slots_exhau
 export interface DesignFileFailure {
   ok: false;
   error: DesignFileError;
-  /** Schema issues as "path: reason", capped. Only a schema failure carries them. */
+  /** From schemaIssues. Only a schema failure carries them. */
   issues?: string[];
 }
 

packages/core/src/lib/schemaIssues.test.ts

@@ -0,0 +1,74 @@
+import { describe, it, expect } from "vitest";
+import { z } from "zod";
+import { schemaIssues } from "./schemaIssues";
+
+const leaf = z.object({ id: z.string(), x: z.number(), y: z.number(), rotation: z.number(), props: z.object({ content: z.string() }) });
+const schema = z.object({
+  label: z.object({ widthMm: z.number(), heightMm: z.number(), dpmm: z.number() }),
+  pages: z.array(z.object({ objects: z.array(z.union([leaf, z.object({ id: z.string(), children: z.array(leaf) })])) })),
+});
+
+const label = { widthMm: 1, heightMm: 1, dpmm: 1 };
+/** Four issues: id, x, y, props.content. */
+const broken = { id: 1, x: "a", y: "b", rotation: 0, props: { content: 1 } };
+
+const issuesOf = (value: unknown) => {
+  const parsed = schema.safeParse(value);
+  if (parsed.success) throw new Error("fixture parses");
+  return schemaIssues(parsed.error, "design");
+};
+const paths = (issues: string[]) => issues.map((line) => line.split(":")[0]);
+
+describe("schemaIssues", () => {
+  it("lists one cause per broken object first and admits what it cut", () => {
+    const issues = issuesOf({ label: { ...label, widthMm: "wide" }, pages: [{ objects: [broken, broken, broken, broken, broken, broken] }] });
+    expect(paths(issues)).toEqual([
+      "label.widthMm",
+      "pages.0.objects.0.id",
+      "pages.0.objects.1.id",
+      "pages.0.objects.2.id",
+      "pages.0.objects.3.id",
+      "and 20 more issues",
+    ]);
+  });
+
+  it("names every broken object before any object's second field", () => {
+    const issues = issuesOf({ label: { widthMm: "a", heightMm: "b", dpmm: "c" }, pages: [{ objects: [broken, broken] }] });
+    expect(paths(issues)).toEqual([
+      "label.widthMm",
+      "pages.0.objects.0.id",
+      "pages.0.objects.1.id",
+      "label.heightMm",
+      "label.dpmm",
+      "and 6 more issues",
+    ]);
+  });
+
+  it("keys a group's children as elements of their own", () => {
+    const issues = issuesOf({ label, pages: [{ objects: [{ id: "g", children: [broken, broken] }] }] });
+    expect(paths(issues)).toEqual([
+      "pages.0.objects.0.children.0.id",
+      "pages.0.objects.0.children.1.id",
+      "pages.0.objects.0.children.0.x",
+      "pages.0.objects.0.children.0.y",
+      "pages.0.objects.0.children.0.props.content",
+      "and 3 more issues",
+    ]);
+  });
+
+  it("counts a single hidden issue in the singular", () => {
+    const issues = issuesOf({ label: { ...label, widthMm: "a", heightMm: "b" }, pages: [{ objects: [broken] }] });
+    expect(issues[5]).toBe("and 1 more issue");
+  });
+
+  it("fills the free places with an object's other fields, without a marker", () => {
+    const issues = issuesOf({ label, pages: [{ objects: [broken] }] });
+    expect(paths(issues)).toEqual(["pages.0.objects.0.id", "pages.0.objects.0.x", "pages.0.objects.0.y", "pages.0.objects.0.props.content"]);
+  });
+
+  it("names a top-level failure by its root", () => {
+    const parsed = z.object({ a: z.number() }).safeParse(5);
+    if (parsed.success) throw new Error("fixture parses");
+    expect(schemaIssues(parsed.error, "design")).toEqual([expect.stringMatching(/^design: /)]);
+  });
+});

packages/core/src/lib/schemaIssues.ts

@@ -16,9 +16,26 @@ function flattenIssues(issues: readonly z.core.$ZodIssue[], prefix: readonly Pro
   });
 }
 
-/** Schema issues as "path: reason", the first few. `root` names a failure at the top level. */
+/** The containing array element, else the parent object, so a broken one costs one slot, not one per field. */
+function issueKey(path: readonly PropertyKey[]): string {
+  const lastIndex = path.reduce<number>((last, seg, i) => (typeof seg === "number" ? i : last), -1);
+  return (lastIndex >= 0 ? path.slice(0, lastIndex + 1) : path.slice(0, -1)).map(String).join(".");
+}
+
+/** Schema issues as "path: reason", the first few, one per element first. A cut list ends with how many it hides. */
 export function schemaIssues(error: z.ZodError, root: string): string[] {
-  return flattenIssues(error.issues)
-    .slice(0, MAX_ISSUES)
-    .map(({ path, message }) => `${path.map(String).join(".") || root}: ${message}`);
+  const all = flattenIssues(error.issues);
+  const seen = new Set<string>();
+  const firsts: typeof all = [];
+  const rest: typeof all = [];
+  for (const issue of all) {
+    const key = issueKey(issue.path);
+    (seen.has(key) ? rest : firsts).push(issue);
+    seen.add(key);
+  }
+  const ordered = [...firsts, ...rest];
+  const shown = ordered.slice(0, MAX_ISSUES).map(({ path, message }) => `${path.map(String).join(".") || root}: ${message}`);
+  const hidden = all.length - shown.length;
+  if (hidden === 0) return shown;
+  return [...shown, hidden === 1 ? "and 1 more issue" : `and ${hidden} more issues`];
 }

packages/core/src/types/LabelObject.ts

@@ -6,7 +6,8 @@ export const labelObjectBaseSchema = z.object({
   type: z.string(),
   x: z.number(),
   y: z.number(),
-  rotation: z.number(),
+  // props.rotation drives orientation, so a file that omits or mangles this still opens.
+  rotation: z.number().default(0).catch(0),
   /** 'FT' = field typeset (baseline), 'FO' = field origin (top-left). Defaults to 'FO'. */
   positionType: z.enum(['FO', 'FT']).optional(),
   /** ^FO/^FT z-justification (spec p.201/205): 'R' = right edge (z=1), absent

packages/mcp-server/src/openInApp.test.ts

@@ -5,7 +5,7 @@ import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
 import { buildServer } from "./server";
 import { openInApp } from "./tools";
 import { resolveDraftReceipt } from "./appBridge";
-import { designFile } from "./testFixtures";
+import { designFile, textObject } from "./testFixtures";
 import { CURRENT_DESIGN_SCHEMA_VERSION } from "@zplab/core/lib/designFile";
 
 async function connect(server: ReturnType<typeof buildServer>): Promise<Client> {
@@ -94,6 +94,13 @@ describe("openInApp validation", () => {
     expect(result.designFile).toEqual({ ...designFile, schemaVersion: CURRENT_DESIGN_SCHEMA_VERSION });
   });
 
+  it("accepts an envelope built the way get_schema describes, without a top-level rotation", () => {
+    const withoutRotation = JSON.parse(JSON.stringify(designFile)) as { pages: { objects: { rotation?: unknown }[] }[] };
+    for (const o of withoutRotation.pages[0]!.objects) delete o.rotation;
+    withoutRotation.pages[0]!.objects.push({ ...textObject("n", "north"), rotation: "N" });
+    expect(openInApp(withoutRotation).ok).toBe(true);
+  });
+
   it("returns errors for a malformed design file", () => {
     const result = openInApp({ schemaVersion: 3, label: { widthMm: 10 } });
     expect(result.ok).toBe(false);

src/lib/designFile.test.ts

@@ -654,6 +654,23 @@ describe('parseDesignFile', () => {
     expect(result.value.pages[0]?.objects).toEqual(designWithGroups);
   });
 
+  it('opens an object whose top-level rotation is missing or mangled, as 0', () => {
+    const text = JSON.stringify({
+      schemaVersion: 6,
+      label: { widthMm: 100, heightMm: 60, dpmm: 8 },
+      pages: [{ objects: [
+        { id: 'a', type: 'text', x: 0, y: 0, props: { content: 'x', fontHeight: 30, fontWidth: 0, rotation: 'R' } },
+        { id: 'b', type: 'text', x: 0, y: 0, rotation: 'N', props: { content: 'y', fontHeight: 30, fontWidth: 0, rotation: 'N' } },
+      ] }],
+    });
+    const result = parseDesignFile(text);
+    expect(result.ok).toBe(true);
+    if (!result.ok) return;
+    expect(result.value.pages[0]?.objects.map((o) => o.rotation)).toEqual([0, 0]);
+    expect((result.value.pages[0]?.objects[0] as { props: { rotation: string } }).props.rotation).toBe('R');
+    expect(JSON.parse(serializeDesign(result.value.label, result.value.pages)).pages[0].objects[0].rotation).toBe(0);
+  });
+
   it('rejects a leaf object that is missing its props', () => {
     const malformed = JSON.stringify({
       label: { widthMm: 100, heightMm: 60, dpmm: 8 },