Skip to content

Commit 848ce01

Browse files
authored
fix(mcp): read a design file handed over as a JSON string (#458)
A partial variable in an envelope is completed by create_draft's builder.
1 parent b6c6842 commit 848ce01

7 files changed

Lines changed: 230 additions & 37 deletions

File tree

‎packages/core/src/lib/designInput.ts‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,26 +213,23 @@ export function duplicateVariableIssue(variables: readonly Variable[]): string |
213213
return null;
214214
}
215215

216-
/** Ids and free ^FN slots for named inputs. Duplicates would merge fields silently, so they are refused. */
217-
export function buildVariables(
216+
/** Ids, free ^FN slots and stripped defaults for named inputs. Duplicates are the caller's policy. */
217+
export function completeVariables(
218218
inputs: readonly VariableInput[],
219219
existing: readonly Variable[] = [],
220220
): { value: Variable[] } | { error: string } {
221-
const names = new Set(existing.map((v) => v.name));
222221
const taken: number[] = [
223222
...existing.map((v) => v.fnNumber),
224223
...inputs.flatMap((v) => (v.fnNumber === undefined ? [] : [v.fnNumber])),
225224
];
226-
const usedIds = new Set(existing.map((v) => v.id));
225+
const usedIds = new Set([...existing.map((v) => v.id), ...inputs.flatMap((v) => (v.id === undefined ? [] : [v.id]))]);
227226
const out: Variable[] = [];
228227
for (const input of inputs) {
229228
// Trimmed like every reader trims, or the stored name could never be addressed again.
230229
const name = input.name.trim();
231230
if (!isValidVariableName(name)) {
232231
return { error: `Invalid variable name: ${JSON.stringify(input.name)}` };
233232
}
234-
if (names.has(name)) return { error: `Duplicate variable name: ${name}` };
235-
names.add(name);
236233
if (input.fnNumber !== undefined && (input.fnNumber < FN_NUMBER_MIN || input.fnNumber > FN_NUMBER_MAX)) {
237234
return { error: `^FN slot must be ${FN_NUMBER_MIN}-${FN_NUMBER_MAX} (got ${input.fnNumber})` };
238235
}
@@ -243,18 +240,35 @@ export function buildVariables(
243240
fnNumber = free;
244241
taken.push(free);
245242
}
246-
let idIndex = existing.length + out.length + 1;
247-
while (usedIds.has(`var-${idIndex}`)) idIndex++;
248-
usedIds.add(`var-${idIndex}`);
243+
let id = input.id;
244+
if (id === undefined) {
245+
let idIndex = existing.length + out.length + 1;
246+
while (usedIds.has(`var-${idIndex}`)) idIndex++;
247+
id = `var-${idIndex}`;
248+
usedIds.add(id);
249+
}
249250
out.push({
250-
id: `var-${idIndex}`,
251+
id,
251252
name,
252253
fnNumber,
253254
// A default carrying its own «…» would resolve in preview and emit verbatim, see stripMarkerDelimiters.
254255
defaultValue: stripMarkerDelimiters(input.defaultValue ?? ""),
255256
...(input.comment !== undefined ? { comment: input.comment } : {}),
256257
});
257258
}
258-
const dup = duplicateVariableIssue([...existing, ...out]);
259-
return dup === null ? { value: out } : { error: dup };
259+
return { value: out };
260+
}
261+
262+
/** create_draft's variables: completed, then duplicates refused, since they would merge fields silently. */
263+
export function buildVariables(
264+
inputs: readonly VariableInput[],
265+
existing: readonly Variable[] = [],
266+
): { value: Variable[] } | { error: string } {
267+
const built = completeVariables(inputs, existing);
268+
if ("error" in built) return built;
269+
const all = [...existing, ...built.value];
270+
const name = all.map((v) => v.name).find((n, i, names) => names.indexOf(n) !== i);
271+
if (name !== undefined) return { error: `Duplicate variable name: ${name}` };
272+
const dup = duplicateVariableIssue(all);
273+
return dup === null ? built : { error: dup };
260274
}

‎packages/core/src/types/Variable.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ export const variableSchema = z.object({
1717
export type Variable = z.infer<typeof variableSchema>;
1818

1919
export interface VariableInput {
20+
/** Kept when given: a file's column mapping addresses the variable by it. */
21+
id?: string;
2022
name: string;
2123
defaultValue?: string;
2224
/** Explicit slot. When omitted, the store assigns the next free number. */

‎packages/mcp-server/src/boundary.ts‎

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import {
1010
} from "@zplab/core/lib/designFile";
1111
import {
1212
buildVariables,
13+
completeVariables,
1314
duplicateVariableIssue,
14-
freeId,
1515
normalizeLeaves,
1616
propIssues,
1717
RAW_GRAPHIC_PROPS,
@@ -25,7 +25,7 @@ import { designSizeIssue } from "@zplab/core/lib/designLimits";
2525
import { getAllLeaves, walkObjects, type LabelObject } from "@zplab/core/types/Group";
2626
import { errorMessage } from "@zplab/core/lib/errorMessage";
2727
import { DPMM_VALUES, isDpmm, type DeviceFontLabel, type Dpmm, type LabelConfig } from "@zplab/core/types/LabelConfig";
28-
import type { Variable, VariableInput } from "@zplab/core/types/Variable";
28+
import { variableSchema, type Variable, type VariableInput } from "@zplab/core/types/Variable";
2929

3030
// Re-exported so the tools keep their names.
3131
export { buildVariables, propIssues, typeIssues };
@@ -52,7 +52,7 @@ export const variableInputSchema: z.ZodType<VariableInput> = z.object({
5252
fnNumber: z.number().int().optional(),
5353
comment: z.string().optional(),
5454
});
55-
export type VariableInputJson = VariableInput;
55+
export type VariableInputJson = Omit<VariableInput, "id">;
5656

5757
export const createDraftShape = {
5858
widthMm: z.number().positive(),
@@ -80,7 +80,12 @@ export interface DesignFileJson {
8080
variables?: Variable[];
8181
}
8282

83-
export const designFileEnvelopeSchema = z.object({ designFile: z.record(z.string(), z.unknown()) });
83+
/** Smaller models tend to hand nested JSON over as a string, so both forms are the same design. */
84+
export const designFileInputSchema = z
85+
.union([z.record(z.string(), z.unknown()), z.string()], { error: "designFile must be the design object or its JSON as a string" })
86+
.describe("The design file as an object, or the same JSON as a string.");
87+
88+
export const designFileEnvelopeSchema = z.object({ designFile: designFileInputSchema });
8489
/** export_zpl: `metadata` keeps ZPLab's ^FX comments for a lossless re-import. */
8590
export const exportZplInputSchema = designFileEnvelopeSchema.extend({ metadata: z.boolean().optional() });
8691

@@ -130,28 +135,56 @@ export function duplicateVariableError(variables: readonly Variable[]): ToolErro
130135
return issue === null ? null : { ok: false, errors: [issue] };
131136
}
132137

133-
/** The model addresses variables by name, so a missing id is filled in. Object ids stay
134-
* required: patch_design addresses objects by id. */
135-
function withVariableIds(designFile: unknown): unknown {
136-
if (!designFile || typeof designFile !== "object") return designFile;
137-
const { variables } = designFile as { variables?: unknown };
138-
if (!Array.isArray(variables)) return designFile;
139-
const taken = new Set<string>();
140-
for (const v of variables) {
141-
if (v && typeof v === "object" && typeof (v as { id?: unknown }).id === "string") taken.add((v as { id: string }).id);
142-
}
143-
const filled = variables.map((v) => {
144-
if (!v || typeof v !== "object" || (v as { id?: unknown }).id !== undefined) return v;
145-
const id = freeId("var", taken);
146-
taken.add(id);
147-
return { ...(v as object), id };
138+
const isRecord = (v: unknown): v is Record<string, unknown> => typeof v === "object" && v !== null;
139+
const isComplete = (v: unknown): v is Variable => variableSchema.safeParse(v).success;
140+
const isPartial = (v: unknown): v is Record<string, unknown> & { name: string } =>
141+
isRecord(v) && typeof v.name === "string" && !isComplete(v);
142+
const typed = <T>(v: unknown, type: "string" | "number"): T | undefined => (typeof v === type ? (v as T) : undefined);
143+
144+
/** A partial variable is completed like create_draft's. A mistyped field comes back as sent, so the
145+
* schema names it alone. Duplicates go to the loader's repair. */
146+
function withCompleteVariables(designFile: unknown): { ok: true; value: unknown } | ToolError {
147+
if (!isRecord(designFile) || !Array.isArray(designFile.variables)) return { ok: true, value: designFile };
148+
const { variables } = designFile;
149+
const partial = variables.filter(isPartial);
150+
if (partial.length === 0) return { ok: true, value: designFile };
151+
const inputs = partial.map((v) => ({
152+
name: v.name,
153+
id: typed<string>(v.id, "string"),
154+
fnNumber: typed<number>(v.fnNumber, "number"),
155+
defaultValue: typed<string>(v.defaultValue, "string"),
156+
comment: typed<string>(v.comment, "string"),
157+
}));
158+
const built = completeVariables(inputs, variables.filter(isComplete));
159+
if ("error" in built) return { ok: false, errors: [built.error] };
160+
let next = 0;
161+
const completed = variables.map((v) => {
162+
if (!isPartial(v)) return v;
163+
const full = built.value[next++] as unknown as Record<string, unknown>;
164+
const mistyped = Object.entries(v).filter(([k, x]) => x !== undefined && typeof x !== typeof full[k]);
165+
return { ...full, ...Object.fromEntries(mistyped) };
148166
});
149-
return { ...(designFile as object), variables: filled };
167+
return { ok: true, value: { ...designFile, variables: completed } };
168+
}
169+
170+
function decodeDesignFile(designFile: unknown): { ok: true; value: unknown } | ToolError {
171+
if (typeof designFile !== "string") return { ok: true, value: designFile };
172+
try {
173+
const value: unknown = JSON.parse(designFile);
174+
if (typeof value === "string") return { ok: false, errors: ["designFile: the JSON decodes to another string, send the design once"] };
175+
return { ok: true, value };
176+
} catch (e) {
177+
return { ok: false, errors: [`designFile: not valid JSON, ${errorMessage(e)}`] };
178+
}
150179
}
151180

152181
export function parseEnvelope(designFile: unknown): { ok: true; value: DesignFile } | ToolError {
182+
const input = decodeDesignFile(designFile);
183+
if (!input.ok) return input;
184+
const completed = withCompleteVariables(input.value);
185+
if (!completed.ok) return completed;
153186
try {
154-
const parsed = parseDesignFile(JSON.stringify(withVariableIds(designFile)));
187+
const parsed = parseDesignFile(JSON.stringify(completed.value));
155188
if (!parsed.ok) return { ok: false, errors: [designFileErrors[parsed.error], ...(parsed.issues ?? [])] };
156189
const issues = labelConfigIssues(parsed.value.label);
157190
if (issues.length > 0) return { ok: false, errors: issues };

‎packages/mcp-server/src/patchOps.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// patch_design: the envelope around the core op reducer, plus the report the agent reads.
22

3-
import { objectInputSchema, pagesSizeError, parseEnvelope, unknownPropNotes, variableInputSchema, type DesignFileJson, type ToolError } from "./boundary.js";
3+
import { designFileInputSchema, objectInputSchema, pagesSizeError, parseEnvelope, unknownPropNotes, variableInputSchema, type DesignFileJson, type ToolError } from "./boundary.js";
44
import { reportFor, type ObjectBounds, type ObjectOverlap, type PreflightWarning } from "./report.js";
55

66
import { z } from "zod";
@@ -58,7 +58,7 @@ export const MAX_PATCH_OPS = 1000;
5858
export const patchOperationsSchema = z.array(patchOpSchema).min(1).max(MAX_PATCH_OPS);
5959

6060
export const patchDesignShape = {
61-
designFile: z.record(z.string(), z.unknown()),
61+
designFile: designFileInputSchema,
6262
operations: patchOperationsSchema,
6363
};
6464

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
22
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
33
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
44
import { buildServer, SERVER_INSTRUCTIONS } from "./server";
5+
import { demoLabel } from "./testFixtures";
56

67
async function connect(hosted = false): Promise<Client> {
78
const [clientT, serverT] = InMemoryTransport.createLinkedPair();
@@ -29,6 +30,32 @@ describe("server handshake", () => {
2930
});
3031
});
3132

33+
describe("the designFile argument", () => {
34+
const envelopeTools = ["validate_draft", "patch_design", "export_zpl", "open_in_app"];
35+
36+
it("is listed as object or string on every envelope tool", async () => {
37+
const { tools } = await (await connect(true)).listTools();
38+
for (const name of envelopeTools) {
39+
const tool = tools.find((t) => t.name === name);
40+
expect(tool, name).toBeDefined();
41+
const schema = tool?.inputSchema as { properties: Record<string, { anyOf?: { type: string }[] }> };
42+
expect(schema.properties.designFile?.anyOf?.map((v) => v.type), name).toEqual(["object", "string"]);
43+
}
44+
});
45+
46+
it("passes the MCP input check as a string and reads the design", async () => {
47+
const client = await connect();
48+
const res = (await client.callTool({
49+
name: "validate_draft",
50+
arguments: { designFile: JSON.stringify(demoLabel) },
51+
})) as { isError?: boolean; content: { text: string }[] };
52+
expect(res.isError).toBeFalsy();
53+
const body = JSON.parse(res.content[0]?.text ?? "{}") as { ok: boolean; bounds: { objectId: string }[] };
54+
expect(body.ok).toBe(true);
55+
expect(body.bounds.map((b) => b.objectId)).toEqual(["titel", "preis", "hinweis", "ean", "rueck"]);
56+
});
57+
});
58+
3259
describe("prepared workflows", () => {
3360
const names = async (hosted: boolean) =>
3461
(await (await connect(hosted)).listPrompts()).prompts.map((p) => p.name);

‎packages/mcp-server/src/testFixtures.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,27 @@ export function textObject(id: string, content: string) {
3333
};
3434
}
3535

36+
/** A label the way a model hand-builds it: bytes a JSON round-trip must carry unchanged. */
37+
export const demoLabel = {
38+
schemaVersion: 6,
39+
label: { widthMm: 100, heightMm: 60, dpmm: 8 },
40+
variables: [
41+
{ name: "bezeichnung", defaultValue: "Bio-Müsli • Früchte" },
42+
{ name: "preis", defaultValue: "4,99 €" },
43+
],
44+
pages: [
45+
{
46+
objects: [
47+
textObject("titel", "«bezeichnung»"),
48+
{ ...textObject("preis", "Preis: «preis»"), y: 60 },
49+
{ ...textObject("hinweis", "• Ohne Zusätze • Äpfel aus der Region"), y: 110 },
50+
{ id: "ean", type: "ean13", x: 10, y: 200, rotation: 0, props: { content: "400638133393", height: 80 } },
51+
],
52+
},
53+
{ objects: [textObject("rueck", "Rückseite: Zutaten • Nährwerte")] },
54+
],
55+
};
56+
3657
/** Minimal valid single-page design file. */
3758
export const designFile = {
3859
schemaVersion: 3,

0 commit comments

Comments
 (0)