diff --git a/templates/design/.agents/skills/design-templates/SKILL.md b/templates/design/.agents/skills/design-templates/SKILL.md index 4117829fa3..882e5c7aca 100644 --- a/templates/design/.agents/skills/design-templates/SKILL.md +++ b/templates/design/.agents/skills/design-templates/SKILL.md @@ -50,9 +50,21 @@ reports `design.createdFromTemplate` on every turn for a template-created design, carrying `lockedDimensions` per screen and `lockedFonts`. Both are captured from the template at copy time, so they describe the template even after the design has been edited many times. Honour them in every -`edit-design` pass: never resize the artboard, change a `canvasFrames` width or -height, switch the primary viewport, or substitute a typeface to fit new -content. +`edit-design` pass: never change them as a side effect of another request — no +resizing the artboard, changing a `canvasFrames` width or height, switching the +primary viewport, or substituting a typeface to fit new content. + +## Changing The Size On Purpose + +An explicit request for a different size wins over the template baseline. Apply +it with `update-design` `dataOperations`, setting `canvasFrames.`, and +tell the user the design now differs from its template. + +Pass every geometry value as a JSON number. `canvasFrames` keeps only finite +numbers on read, so `"width": "595"` writes successfully and then reads back as +*no* width — the update looks applied and silently is not. `update-design` now +rejects that write, and the error names the field; resend it as +`"width": 595`. Call `get-design-template --designId=""` when you need more than those two facts — the template's original markup, or its locked layers — for example diff --git a/templates/design/actions/update-design.spec.ts b/templates/design/actions/update-design.spec.ts index 03babb781b..94958d0a32 100644 --- a/templates/design/actions/update-design.spec.ts +++ b/templates/design/actions/update-design.spec.ts @@ -201,6 +201,57 @@ describe("update-design data concurrency", () => { mocks.assertAccess.mockResolvedValue(undefined); }); + it("rejects string canvas dimensions that would read back empty", async () => { + const before = mocks.state.row.data; + await expect( + action.run({ + id: "design-1", + dataOperations: [ + { + op: "set", + path: ["canvasFrames", "frame-a"], + value: { x: "651", y: "0", width: "595", height: "842", z: "0" }, + }, + ], + operationSource: "chat-1", + operationRevision: 1, + } as never), + ).rejects.toThrow("must be a finite number of pixels"); + expect(mocks.state.row.data).toBe(before); + }); + + it("rejects a string dimension addressed at the leaf path", async () => { + await expect( + action.run({ + id: "design-1", + dataOperations: [ + { + op: "set", + path: ["canvasFrames", "frame-a", "width"], + value: "595", + }, + ], + } as never), + ).rejects.toThrow("canvasFrames.frame-a.width"); + }); + + it("still accepts numeric canvas dimensions", async () => { + const result = await action.run({ + id: "design-1", + dataOperations: [ + { + op: "set", + path: ["canvasFrames", "frame-a"], + value: { x: 651, y: 0, width: 595, height: 842, z: 0 }, + }, + ], + } as never); + expect(result).toMatchObject({ id: "design-1", updated: true }); + expect( + JSON.parse(String(mocks.state.row.data)).canvasFrames["frame-a"], + ).toEqual({ x: 651, y: 0, width: 595, height: 842, z: 0 }); + }); + it("rejects one ambiguous legacy snapshot instead of silently losing a concurrent frame edit", async () => { mocks.resetReadGate(2); const moveA = { diff --git a/templates/design/actions/update-design.ts b/templates/design/actions/update-design.ts index c96a4cf3c3..6253584303 100644 --- a/templates/design/actions/update-design.ts +++ b/templates/design/actions/update-design.ts @@ -4,6 +4,10 @@ import { and, eq, isNull } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { + canvasFrameGeometryWriteErrors, + isCanvasFrameGeometryKey, +} from "../shared/canvas-frames.js"; const MAX_DATA_CAS_ATTEMPTS = 5; const MAX_DATA_OPERATION_SOURCES = 128; @@ -42,6 +46,49 @@ type DataOperation = z.infer; type DataOperationRevisions = Record; +/** + * Canvas geometry is the one part of `data` where a write that type-checks as + * JSON can still be inert: the readers keep only finite numbers, so string + * dimensions persist and then read back as no dimensions at all, and the + * caller is told the resize succeeded. Reject at the write instead. + */ +function canvasFrameWriteErrors(operations: DataOperation[]): string[] { + return operations.flatMap((operation) => { + if (operation.op !== "set") return []; + const [root, fileId, field] = operation.path; + if (root !== "canvasFrames") return []; + + if (fileId === undefined) { + if (!isRecord(operation.value)) { + return ["canvasFrames must be an object keyed by design file id"]; + } + return Object.entries(operation.value).flatMap(([id, frame]) => + canvasFrameGeometryWriteErrors(frame, `canvasFrames.${id}`), + ); + } + if (field === undefined) { + return canvasFrameGeometryWriteErrors( + operation.value, + `canvasFrames.${fileId}`, + ); + } + if (!isCanvasFrameGeometryKey(field)) return []; + return canvasFrameGeometryWriteErrors( + { [field]: operation.value }, + `canvasFrames.${fileId}`, + ); + }); +} + +function assertCanvasFramesWritable(operations: DataOperation[]): void { + const errors = canvasFrameWriteErrors(operations); + if (errors.length === 0) return; + throw new Error( + `Canvas frame update rejected because the values would not persist: ${errors.join("; ")}. ` + + "Send canvas dimensions as JSON numbers, not strings.", + ); +} + /** * Normalize affected-row metadata from every createGetDb backend: libSQL, * PGlite, Neon, postgres.js, better-sqlite3, and D1. @@ -317,6 +364,8 @@ export default defineAction({ return { id, updated: true }; } + if (dataOperations) assertCanvasFramesWritable(dataOperations); + const maxAttempts = dataOperations ? MAX_DATA_CAS_ATTEMPTS : 1; for (let attempt = 0; attempt < maxAttempts; attempt += 1) { const [existing] = await db diff --git a/templates/design/actions/view-screen.ts b/templates/design/actions/view-screen.ts index 28712f6d8e..f395a54b7e 100644 --- a/templates/design/actions/view-screen.ts +++ b/templates/design/actions/view-screen.ts @@ -351,7 +351,8 @@ export default defineAction({ note: "The screens below are edited copies of this template, so their current content no longer shows what the template specified. " + "The dimensions and fonts above come from the template and stay authoritative for every request, including this one: keep each screen at exactly those dimensions and keep those font families. " + - "Do not resize the artboard, change canvasFrames width or height, switch the primary viewport, or substitute a typeface to fit new content. " + + "Never change them as a side effect of another request — no resizing the artboard, changing canvasFrames width or height, switching the primary viewport, or substituting a typeface to fit new content. " + + "When the user explicitly asks for a different size, that request wins: change it with `update-design` dataOperations on `canvasFrames.`, passing width and height as JSON numbers, and say that the design now differs from its template. " + `Refine with edit-design; do not call generate-design. Call \`get-design-template --designId="${designId}"\` when you need the template's original markup or locked layers.`, }; } diff --git a/templates/design/shared/canvas-frame-writes.test.ts b/templates/design/shared/canvas-frame-writes.test.ts new file mode 100644 index 0000000000..3b2a32efd0 --- /dev/null +++ b/templates/design/shared/canvas-frame-writes.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; + +import { + canvasFrameGeometryWriteErrors, + isCanvasFrameGeometryKey, + parseCanvasFrameGeometryById, +} from "./canvas-frames.js"; + +describe("canvas frame write validation", () => { + it("rejects the string-valued frame that reads back as no dimensions", () => { + const frame = { x: "651", y: "0", width: "595", height: "842", z: "0" }; + + // Without the guard this write persists and then parses to an empty + // frame, which is why a resize could report success and change nothing. + expect(parseCanvasFrameGeometryById({ screen: frame })).toEqual({ + screen: {}, + }); + + const errors = canvasFrameGeometryWriteErrors(frame, "canvasFrames.screen"); + expect(errors).toHaveLength(5); + expect(errors[0]).toBe( + 'canvasFrames.screen.x must be a finite number of pixels, received string "651"', + ); + expect(errors).toContain( + 'canvasFrames.screen.width must be a finite number of pixels, received string "595"', + ); + }); + + it("accepts numeric geometry and ignores absent fields", () => { + expect( + canvasFrameGeometryWriteErrors( + { x: 651, y: 0, width: 595, height: 842 }, + "canvasFrames.screen", + ), + ).toEqual([]); + expect( + canvasFrameGeometryWriteErrors({ width: 595 }, "canvasFrames.screen"), + ).toEqual([]); + }); + + it("rejects non-finite numbers and non-object frames", () => { + expect( + canvasFrameGeometryWriteErrors( + { width: Number.NaN }, + "canvasFrames.screen", + ), + ).toHaveLength(1); + expect(canvasFrameGeometryWriteErrors(null, "canvasFrames.screen")).toEqual( + [ + "canvasFrames.screen must be an object of numeric pixel values, received null", + ], + ); + }); + + it("knows which keys carry geometry", () => { + expect(isCanvasFrameGeometryKey("width")).toBe(true); + expect(isCanvasFrameGeometryKey("rotation")).toBe(true); + expect(isCanvasFrameGeometryKey("label")).toBe(false); + }); +}); diff --git a/templates/design/shared/canvas-frames.ts b/templates/design/shared/canvas-frames.ts index e87533f1c7..1c1ad32acb 100644 --- a/templates/design/shared/canvas-frames.ts +++ b/templates/design/shared/canvas-frames.ts @@ -29,6 +29,49 @@ function finiteNumber(value: unknown): number | undefined { : undefined; } +export function isCanvasFrameGeometryKey(key: string): boolean { + return (CANVAS_FRAME_GEOMETRY_KEYS as readonly string[]).includes(key); +} + +function describeValue(value: unknown): string { + if (typeof value === "string") return "string " + JSON.stringify(value); + if (value === null) return "null"; + if (typeof value === "number") return String(value); + return typeof value; +} + +/** + * Reads drop any geometry field that is not a finite number, so a write + * carrying "595" persists happily and then renders as no dimension at all — + * indistinguishable from a successful resize. Writers must fail here rather + * than store a value this module will silently discard on the way back out. + */ +export function canvasFrameGeometryWriteErrors( + frame: unknown, + label: string, +): string[] { + if (!frame || typeof frame !== "object" || Array.isArray(frame)) { + return [ + label + + " must be an object of numeric pixel values, received " + + describeValue(frame), + ]; + } + const raw = frame as Record; + return CANVAS_FRAME_GEOMETRY_KEYS.flatMap((key) => { + if (!(key in raw) || raw[key] === undefined) return []; + return finiteNumber(raw[key]) === undefined + ? [ + label + + "." + + key + + " must be a finite number of pixels, received " + + describeValue(raw[key]), + ] + : []; + }); +} + export function parseCanvasFrameGeometry( value: unknown, ): CanvasFrameGeometry | null {