fix(mcp): read a design file handed over as a JSON string

packages/core/src/lib/designInput.ts

@@ -213,26 +213,23 @@ export function duplicateVariableIssue(variables: readonly Variable[]): string |
   return null;
 }
 
-/** Ids and free ^FN slots for named inputs. Duplicates would merge fields silently, so they are refused. */
-export function buildVariables(
+/** Ids, free ^FN slots and stripped defaults for named inputs. Duplicates are the caller's policy. */
+export function completeVariables(
   inputs: readonly VariableInput[],
   existing: readonly Variable[] = [],
 ): { value: Variable[] } | { error: string } {
-  const names = new Set(existing.map((v) => v.name));
   const taken: number[] = [
     ...existing.map((v) => v.fnNumber),
     ...inputs.flatMap((v) => (v.fnNumber === undefined ? [] : [v.fnNumber])),
   ];
-  const usedIds = new Set(existing.map((v) => v.id));
+  const usedIds = new Set([...existing.map((v) => v.id), ...inputs.flatMap((v) => (v.id === undefined ? [] : [v.id]))]);
   const out: Variable[] = [];
   for (const input of inputs) {
     // Trimmed like every reader trims, or the stored name could never be addressed again.
     const name = input.name.trim();
     if (!isValidVariableName(name)) {
       return { error: `Invalid variable name: ${JSON.stringify(input.name)}` };
     }
-    if (names.has(name)) return { error: `Duplicate variable name: ${name}` };
-    names.add(name);
     if (input.fnNumber !== undefined && (input.fnNumber < FN_NUMBER_MIN || input.fnNumber > FN_NUMBER_MAX)) {
       return { error: `^FN slot must be ${FN_NUMBER_MIN}-${FN_NUMBER_MAX} (got ${input.fnNumber})` };
     }
@@ -243,18 +240,35 @@ export function buildVariables(
       fnNumber = free;
       taken.push(free);
     }
-    let idIndex = existing.length + out.length + 1;
-    while (usedIds.has(`var-${idIndex}`)) idIndex++;
-    usedIds.add(`var-${idIndex}`);
+    let id = input.id;
+    if (id === undefined) {
+      let idIndex = existing.length + out.length + 1;
+      while (usedIds.has(`var-${idIndex}`)) idIndex++;
+      id = `var-${idIndex}`;
+      usedIds.add(id);
+    }
     out.push({
-      id: `var-${idIndex}`,
+      id,
       name,
       fnNumber,
       // A default carrying its own «…» would resolve in preview and emit verbatim, see stripMarkerDelimiters.
       defaultValue: stripMarkerDelimiters(input.defaultValue ?? ""),
       ...(input.comment !== undefined ? { comment: input.comment } : {}),
     });
   }
-  const dup = duplicateVariableIssue([...existing, ...out]);
-  return dup === null ? { value: out } : { error: dup };
+  return { value: out };
+}
+
+/** create_draft's variables: completed, then duplicates refused, since they would merge fields silently. */
+export function buildVariables(
+  inputs: readonly VariableInput[],
+  existing: readonly Variable[] = [],
+): { value: Variable[] } | { error: string } {
+  const built = completeVariables(inputs, existing);
+  if ("error" in built) return built;
+  const all = [...existing, ...built.value];
+  const name = all.map((v) => v.name).find((n, i, names) => names.indexOf(n) !== i);
+  if (name !== undefined) return { error: `Duplicate variable name: ${name}` };
+  const dup = duplicateVariableIssue(all);
+  return dup === null ? built : { error: dup };
 }

packages/core/src/types/Variable.ts

@@ -17,6 +17,8 @@ export const variableSchema = z.object({
 export type Variable = z.infer<typeof variableSchema>;
 
 export interface VariableInput {
+  /** Kept when given: a file's column mapping addresses the variable by it. */
+  id?: string;
   name: string;
   defaultValue?: string;
   /** Explicit slot. When omitted, the store assigns the next free number. */

packages/mcp-server/src/boundary.ts

@@ -10,8 +10,8 @@ import {
 } from "@zplab/core/lib/designFile";
 import {
   buildVariables,
+  completeVariables,
   duplicateVariableIssue,
-  freeId,
   normalizeLeaves,
   propIssues,
   RAW_GRAPHIC_PROPS,
@@ -25,7 +25,7 @@ import { designSizeIssue } from "@zplab/core/lib/designLimits";
 import { getAllLeaves, walkObjects, type LabelObject } from "@zplab/core/types/Group";
 import { errorMessage } from "@zplab/core/lib/errorMessage";
 import { DPMM_VALUES, isDpmm, type DeviceFontLabel, type Dpmm, type LabelConfig } from "@zplab/core/types/LabelConfig";
-import type { Variable, VariableInput } from "@zplab/core/types/Variable";
+import { variableSchema, type Variable, type VariableInput } from "@zplab/core/types/Variable";
 
 // Re-exported so the tools keep their names.
 export { buildVariables, propIssues, typeIssues };
@@ -52,7 +52,7 @@ export const variableInputSchema: z.ZodType<VariableInput> = z.object({
   fnNumber: z.number().int().optional(),
   comment: z.string().optional(),
 });
-export type VariableInputJson = VariableInput;
+export type VariableInputJson = Omit<VariableInput, "id">;
 
 export const createDraftShape = {
   widthMm: z.number().positive(),
@@ -80,7 +80,12 @@ export interface DesignFileJson {
   variables?: Variable[];
 }
 
-export const designFileEnvelopeSchema = z.object({ designFile: z.record(z.string(), z.unknown()) });
+/** Smaller models tend to hand nested JSON over as a string, so both forms are the same design. */
+export const designFileInputSchema = z
+  .union([z.record(z.string(), z.unknown()), z.string()], { error: "designFile must be the design object or its JSON as a string" })
+  .describe("The design file as an object, or the same JSON as a string.");
+
+export const designFileEnvelopeSchema = z.object({ designFile: designFileInputSchema });
 /** export_zpl: `metadata` keeps ZPLab's ^FX comments for a lossless re-import. */
 export const exportZplInputSchema = designFileEnvelopeSchema.extend({ metadata: z.boolean().optional() });
 
@@ -130,28 +135,56 @@ export function duplicateVariableError(variables: readonly Variable[]): ToolErro
   return issue === null ? null : { ok: false, errors: [issue] };
 }
 
-/** The model addresses variables by name, so a missing id is filled in. Object ids stay
- *  required: patch_design addresses objects by id. */
-function withVariableIds(designFile: unknown): unknown {
-  if (!designFile || typeof designFile !== "object") return designFile;
-  const { variables } = designFile as { variables?: unknown };
-  if (!Array.isArray(variables)) return designFile;
-  const taken = new Set<string>();
-  for (const v of variables) {
-    if (v && typeof v === "object" && typeof (v as { id?: unknown }).id === "string") taken.add((v as { id: string }).id);
-  }
-  const filled = variables.map((v) => {
-    if (!v || typeof v !== "object" || (v as { id?: unknown }).id !== undefined) return v;
-    const id = freeId("var", taken);
-    taken.add(id);
-    return { ...(v as object), id };
+const isRecord = (v: unknown): v is Record<string, unknown> => typeof v === "object" && v !== null;
+const isComplete = (v: unknown): v is Variable => variableSchema.safeParse(v).success;
+const isPartial = (v: unknown): v is Record<string, unknown> & { name: string } =>
+  isRecord(v) && typeof v.name === "string" && !isComplete(v);
+const typed = <T>(v: unknown, type: "string" | "number"): T | undefined => (typeof v === type ? (v as T) : undefined);
+
+/** A partial variable is completed like create_draft's. A mistyped field comes back as sent, so the
+ *  schema names it alone. Duplicates go to the loader's repair. */
+function withCompleteVariables(designFile: unknown): { ok: true; value: unknown } | ToolError {
+  if (!isRecord(designFile) || !Array.isArray(designFile.variables)) return { ok: true, value: designFile };
+  const { variables } = designFile;
+  const partial = variables.filter(isPartial);
+  if (partial.length === 0) return { ok: true, value: designFile };
+  const inputs = partial.map((v) => ({
+    name: v.name,
+    id: typed<string>(v.id, "string"),
+    fnNumber: typed<number>(v.fnNumber, "number"),
+    defaultValue: typed<string>(v.defaultValue, "string"),
+    comment: typed<string>(v.comment, "string"),
+  }));
+  const built = completeVariables(inputs, variables.filter(isComplete));
+  if ("error" in built) return { ok: false, errors: [built.error] };
+  let next = 0;
+  const completed = variables.map((v) => {
+    if (!isPartial(v)) return v;
+    const full = built.value[next++] as unknown as Record<string, unknown>;
+    const mistyped = Object.entries(v).filter(([k, x]) => x !== undefined && typeof x !== typeof full[k]);
+    return { ...full, ...Object.fromEntries(mistyped) };
   });
-  return { ...(designFile as object), variables: filled };
+  return { ok: true, value: { ...designFile, variables: completed } };
+}
+
+function decodeDesignFile(designFile: unknown): { ok: true; value: unknown } | ToolError {
+  if (typeof designFile !== "string") return { ok: true, value: designFile };
+  try {
+    const value: unknown = JSON.parse(designFile);
+    if (typeof value === "string") return { ok: false, errors: ["designFile: the JSON decodes to another string, send the design once"] };
+    return { ok: true, value };
+  } catch (e) {
+    return { ok: false, errors: [`designFile: not valid JSON, ${errorMessage(e)}`] };
+  }
 }
 
 export function parseEnvelope(designFile: unknown): { ok: true; value: DesignFile } | ToolError {
+  const input = decodeDesignFile(designFile);
+  if (!input.ok) return input;
+  const completed = withCompleteVariables(input.value);
+  if (!completed.ok) return completed;
   try {
-    const parsed = parseDesignFile(JSON.stringify(withVariableIds(designFile)));
+    const parsed = parseDesignFile(JSON.stringify(completed.value));
     if (!parsed.ok) return { ok: false, errors: [designFileErrors[parsed.error], ...(parsed.issues ?? [])] };
     const issues = labelConfigIssues(parsed.value.label);
     if (issues.length > 0) return { ok: false, errors: issues };

packages/mcp-server/src/patchOps.ts

@@ -1,6 +1,6 @@
 // patch_design: the envelope around the core op reducer, plus the report the agent reads.
 
-import { objectInputSchema, pagesSizeError, parseEnvelope, unknownPropNotes, variableInputSchema, type DesignFileJson, type ToolError } from "./boundary.js";
+import { designFileInputSchema, objectInputSchema, pagesSizeError, parseEnvelope, unknownPropNotes, variableInputSchema, type DesignFileJson, type ToolError } from "./boundary.js";
 import { reportFor, type ObjectBounds, type ObjectOverlap, type PreflightWarning } from "./report.js";
 
 import { z } from "zod";
@@ -58,7 +58,7 @@ export const MAX_PATCH_OPS = 1000;
 export const patchOperationsSchema = z.array(patchOpSchema).min(1).max(MAX_PATCH_OPS);
 
 export const patchDesignShape = {
-  designFile: z.record(z.string(), z.unknown()),
+  designFile: designFileInputSchema,
   operations: patchOperationsSchema,
 };
 

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

@@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
 import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
 import { buildServer, SERVER_INSTRUCTIONS } from "./server";
+import { demoLabel } from "./testFixtures";
 
 async function connect(hosted = false): Promise<Client> {
   const [clientT, serverT] = InMemoryTransport.createLinkedPair();
@@ -29,6 +30,32 @@ describe("server handshake", () => {
   });
 });
 
+describe("the designFile argument", () => {
+  const envelopeTools = ["validate_draft", "patch_design", "export_zpl", "open_in_app"];
+
+  it("is listed as object or string on every envelope tool", async () => {
+    const { tools } = await (await connect(true)).listTools();
+    for (const name of envelopeTools) {
+      const tool = tools.find((t) => t.name === name);
+      expect(tool, name).toBeDefined();
+      const schema = tool?.inputSchema as { properties: Record<string, { anyOf?: { type: string }[] }> };
+      expect(schema.properties.designFile?.anyOf?.map((v) => v.type), name).toEqual(["object", "string"]);
+    }
+  });
+
+  it("passes the MCP input check as a string and reads the design", async () => {
+    const client = await connect();
+    const res = (await client.callTool({
+      name: "validate_draft",
+      arguments: { designFile: JSON.stringify(demoLabel) },
+    })) as { isError?: boolean; content: { text: string }[] };
+    expect(res.isError).toBeFalsy();
+    const body = JSON.parse(res.content[0]?.text ?? "{}") as { ok: boolean; bounds: { objectId: string }[] };
+    expect(body.ok).toBe(true);
+    expect(body.bounds.map((b) => b.objectId)).toEqual(["titel", "preis", "hinweis", "ean", "rueck"]);
+  });
+});
+
 describe("prepared workflows", () => {
   const names = async (hosted: boolean) =>
     (await (await connect(hosted)).listPrompts()).prompts.map((p) => p.name);

packages/mcp-server/src/testFixtures.ts

@@ -33,6 +33,27 @@ export function textObject(id: string, content: string) {
   };
 }
 
+/** A label the way a model hand-builds it: bytes a JSON round-trip must carry unchanged. */
+export const demoLabel = {
+  schemaVersion: 6,
+  label: { widthMm: 100, heightMm: 60, dpmm: 8 },
+  variables: [
+    { name: "bezeichnung", defaultValue: "Bio-Müsli • Früchte" },
+    { name: "preis", defaultValue: "4,99 €" },
+  ],
+  pages: [
+    {
+      objects: [
+        textObject("titel", "«bezeichnung»"),
+        { ...textObject("preis", "Preis: «preis»"), y: 60 },
+        { ...textObject("hinweis", "• Ohne Zusätze • Äpfel aus der Region"), y: 110 },
+        { id: "ean", type: "ean13", x: 10, y: 200, rotation: 0, props: { content: "400638133393", height: 80 } },
+      ],
+    },
+    { objects: [textObject("rueck", "Rückseite: Zutaten • Nährwerte")] },
+  ],
+};
+
 /** Minimal valid single-page design file. */
 export const designFile = {
   schemaVersion: 3,