From 90b608863ea4a9ab0133f43c2d91f025ca2e9b02 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Thu, 3 Sep 2026 08:09:07 +0300 Subject: [PATCH] fix(ai): validate stock photo targets - Reject text, line, structural, and populated container targets before provider requests\n- Preserve closed shape and Boolean geometry support without mutating target bounds\n- Cover accepted and rejected node families with focused tests --- CHANGELOG.md | 1 + packages/core/src/tools/stock-photo.ts | 5 +- packages/core/src/tools/stock-photo/apply.ts | 31 ++- src/app/ai/chat/system-prompt.md | 4 +- tests/engine/tools/stock-photo/apply.test.ts | 191 +++++++++++++++++++ 5 files changed, 221 insertions(+), 11 deletions(-) create mode 100644 tests/engine/tools/stock-photo/apply.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a0ce6e91..e8028980e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ ### Fixed +- Prevent the stock photo tool from replacing text, lines, structural layers, or containers with content while supporting closed shape geometry. - Preserve explicit text alignment metadata on imported Figma vectors across save and reload. - Preserve explicit normal blend modes on imported Figma text and vector nodes across save and reload. diff --git a/packages/core/src/tools/stock-photo.ts b/packages/core/src/tools/stock-photo.ts index faa4bae87..922d19644 100644 --- a/packages/core/src/tools/stock-photo.ts +++ b/packages/core/src/tools/stock-photo.ts @@ -19,8 +19,9 @@ export const stockPhoto = defineTool({ name: 'stock_photo', mutates: true, description: - 'Search stock photos and apply to nodes. Pass a JSON array — all fetched in parallel. ' + - 'Each item: {id, query, index?, orientation?}. Only works on leaf shapes (Rectangle/Ellipse).', + 'Search stock photos and apply to leaf image placeholders or closed area geometry. ' + + 'Pass a JSON array; each item is {id, query, index?, orientation?}. ' + + 'Containers with content, text, lines, and structural nodes are rejected.', params: { requests: { type: 'string', diff --git a/packages/core/src/tools/stock-photo/apply.ts b/packages/core/src/tools/stock-photo/apply.ts index 816ad8599..32722482a 100644 --- a/packages/core/src/tools/stock-photo/apply.ts +++ b/packages/core/src/tools/stock-photo/apply.ts @@ -1,7 +1,22 @@ +import type { SceneNode } from '@open-pencil/scene-graph' + import type { FigmaAPI } from '#core/figma-api' import type { StockPhotoProvider, StockPhotoResult } from './providers' +const STOCK_PHOTO_TARGET_TYPES: ReadonlySet = new Set([ + 'FRAME', + 'RECTANGLE', + 'ROUNDED_RECTANGLE', + 'ELLIPSE', + 'STAR', + 'POLYGON', + 'VECTOR', + 'BOOLEAN_OPERATION', + 'COMPONENT', + 'INSTANCE' +]) + export interface PhotoRequest { id: string query: string @@ -9,10 +24,6 @@ export interface PhotoRequest { orientation?: 'landscape' | 'portrait' | 'square' } -interface NodeWithChildren { - children: unknown[] -} - export interface PhotoResult { id: string photo?: { @@ -33,9 +44,15 @@ export async function applyPhoto( const node = figma.getNodeById(req.id) if (!node) return { id: req.id, error: 'Not found' } - const children = 'children' in node ? (node as NodeWithChildren).children : [] - if (children.length > 0) { - return { id: req.id, error: `"${node.name}" has children — use a leaf shape` } + if (!STOCK_PHOTO_TARGET_TYPES.has(node.type)) { + return { + id: req.id, + error: `"${node.name}" (${node.type}) is not a suitable stock photo target` + } + } + + if (node.type !== 'BOOLEAN_OPERATION' && node.children.length > 0) { + return { id: req.id, error: `"${node.name}" has children — use a leaf image placeholder` } } const perPage = Math.min((req.index ?? 0) + 3, 15) diff --git a/src/app/ai/chat/system-prompt.md b/src/app/ai/chat/system-prompt.md index 12adb3fda..eea90698d 100644 --- a/src/app/ai/chat/system-prompt.md +++ b/src/app/ai/chat/system-prompt.md @@ -96,14 +96,14 @@ No style={{}}, className, CSS. No named colors or rgb(). No percentage values. N # Stock Photos -`stock_photo` places real Pexels images on leaf shapes (Rectangle/Ellipse). Pass a JSON array — **all photos fetched in parallel**: +`stock_photo` places real Pexels images on leaf image placeholders. Prefer Rectangle, Rounded Rectangle, or Ellipse placeholders; closed vector and Boolean geometry are also supported. Pass a JSON array — **all photos fetched in parallel**: ``` stock_photo({ requests: '[{"id":"0:30","query":"wall street trading floor"},{"id":"0:58","query":"AI chip semiconductor"},{"id":"0:65","query":"bank finance credit card"}]' }) ``` - Batch all photos in one call — don't call stock_photo 14 times separately -- Only apply to leaf shapes (Rectangle/Ellipse), NOT to Frames with children +- Only apply to leaf image placeholders, not text, lines, groups, sections, or containers with content - Use descriptive English queries: "aerial city skyline sunset", not "image1" - Orientation: "landscape" (default), "portrait" for tall cards, "square" for avatars - If Pexels key is not configured or returns 401, tell the user to add/check it in AI chat settings. Do NOT fall back to `eval` with manual gradients — leave placeholder colors as-is diff --git a/tests/engine/tools/stock-photo/apply.test.ts b/tests/engine/tools/stock-photo/apply.test.ts new file mode 100644 index 000000000..4260ea0df --- /dev/null +++ b/tests/engine/tools/stock-photo/apply.test.ts @@ -0,0 +1,191 @@ +import { describe, expect, test } from 'bun:test' + +import { FigmaAPI, SceneGraph } from '@open-pencil/core' +import type { NodeType } from '@open-pencil/scene-graph' +import { copyFills } from '@open-pencil/scene-graph/copy' + +import { applyPhoto } from '#core/tools/stock-photo/apply' +import type { StockPhotoProvider } from '#core/tools/stock-photo/providers' + +const PNG_BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) +const PHOTO_URL = `data:image/png;base64,${PNG_BYTES.toBase64()}` + +interface ProviderCall { + query: string + options: { + perPage: number + orientation: 'landscape' | 'portrait' | 'square' + targetDim: number + } +} + +function createProvider(calls: ProviderCall[]): StockPhotoProvider { + return { + name: 'test', + async search(query, options) { + calls.push({ query, options }) + return [ + { + url: PHOTO_URL, + width: 1600, + height: 900, + photographer: 'Test Photographer', + sourceId: 'photo-1' + } + ] + } + } +} + +function setup() { + const graph = new SceneGraph() + const page = graph.getPages()[0] + if (!page) throw new Error('Expected default page') + return { graph, page, figma: new FigmaAPI(graph) } +} + +const AREA_TARGET_TYPES = [ + 'FRAME', + 'RECTANGLE', + 'ROUNDED_RECTANGLE', + 'ELLIPSE', + 'STAR', + 'POLYGON', + 'VECTOR', + 'COMPONENT', + 'INSTANCE' +] as const satisfies readonly NodeType[] + +const UNSUITABLE_TARGET_TYPES = [ + 'CANVAS', + 'GROUP', + 'TEXT', + 'LINE', + 'SECTION', + 'COMPONENT_SET', + 'CONNECTOR', + 'SHAPE_WITH_TEXT' +] as const satisfies readonly NodeType[] + +describe('applyPhoto', () => { + for (const type of AREA_TARGET_TYPES) { + test(`applies a photo to an empty ${type}`, async () => { + const { graph, page, figma } = setup() + const node = graph.createNode(type, page.id, { + name: `${type} target`, + x: 12, + y: 24, + width: 320, + height: 180 + }) + const calls: ProviderCall[] = [] + + const result = await applyPhoto(figma, createProvider(calls), { + id: node.id, + query: 'mountain sunset', + orientation: 'landscape' + }) + + expect(result.error).toBeUndefined() + expect(result.photo).toMatchObject({ sourceId: 'photo-1', provider: 'test' }) + expect(calls).toEqual([ + { + query: 'mountain sunset', + options: { perPage: 3, orientation: 'landscape', targetDim: 320 } + } + ]) + expect(node.fills[0]).toMatchObject({ type: 'IMAGE', imageScaleMode: 'FILL' }) + expect(graph.images.size).toBe(1) + expect({ x: node.x, y: node.y, width: node.width, height: node.height }).toEqual({ + x: 12, + y: 24, + width: 320, + height: 180 + }) + }) + } + + test('applies a photo to Boolean geometry with operand children', async () => { + const { graph, page, figma } = setup() + const operation = graph.createNode('BOOLEAN_OPERATION', page.id, { + name: 'Combined shape', + width: 200, + height: 120 + }) + graph.createNode('RECTANGLE', operation.id) + graph.createNode('ELLIPSE', operation.id) + const calls: ProviderCall[] = [] + + const result = await applyPhoto(figma, createProvider(calls), { + id: operation.id, + query: 'abstract texture' + }) + + expect(result.error).toBeUndefined() + expect(calls).toHaveLength(1) + expect(operation.fills[0]).toMatchObject({ type: 'IMAGE', imageScaleMode: 'FILL' }) + }) + + for (const type of UNSUITABLE_TARGET_TYPES) { + test(`rejects ${type} before searching`, async () => { + const { graph, page, figma } = setup() + const node = + type === 'CANVAS' + ? page + : graph.createNode(type, page.id, { + name: `${type} target`, + fills: [ + { + type: 'SOLID', + color: { r: 1, g: 0, b: 0, a: 1 }, + opacity: 1, + visible: true + } + ] + }) + const originalFills = copyFills(node.fills) + const calls: ProviderCall[] = [] + + const result = await applyPhoto(figma, createProvider(calls), { + id: node.id, + query: 'should not run' + }) + + expect(result.error).toContain(`(${type}) is not a suitable stock photo target`) + expect(calls).toHaveLength(0) + expect(node.fills).toEqual(originalFills) + expect(graph.images.size).toBe(0) + }) + } + + for (const type of ['FRAME', 'COMPONENT', 'INSTANCE'] as const satisfies readonly NodeType[]) { + test(`rejects ${type} content containers before searching`, async () => { + const { graph, page, figma } = setup() + const container = graph.createNode(type, page.id, { + name: `${type} with content`, + fills: [ + { + type: 'SOLID', + color: { r: 0, g: 0, b: 1, a: 1 }, + opacity: 1, + visible: true + } + ] + }) + const child = graph.createNode('RECTANGLE', container.id) + const originalFills = copyFills(container.fills) + const calls: ProviderCall[] = [] + + const result = await applyPhoto(figma, createProvider(calls), { + id: container.id, + query: 'should not run' + }) + + expect(result.error).toBe(`"${container.name}" has children — use a leaf image placeholder`) + expect(calls).toHaveLength(0) + expect(container.fills).toEqual(originalFills) + expect(graph.getNode(child.id)).toBe(child) + expect(graph.images.size).toBe(0) + }) + } +})