Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions templates/design/.agents/skills/design-templates/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<fileId>`, 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="<id>"` when you need more than those two
facts — the template's original markup, or its locked layers — for example
Expand Down
51 changes: 51 additions & 0 deletions templates/design/actions/update-design.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
49 changes: 49 additions & 0 deletions templates/design/actions/update-design.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -42,6 +46,49 @@ type DataOperation = z.infer<typeof dataOperationSchema>;

type DataOperationRevisions = Record<string, number>;

/**
* 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.
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion templates/design/actions/view-screen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.<fileId>`, 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.`,
};
}
Expand Down
60 changes: 60 additions & 0 deletions templates/design/shared/canvas-frame-writes.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
43 changes: 43 additions & 0 deletions templates/design/shared/canvas-frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
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 {
Expand Down
Loading