Sitesmith: self-review render via review_render_upload; one-line-per-node summaries; real-browser check of cleaned output

- The render is uploaded with the new review_render_upload action (backend kind
  review_render, the only image a review job accepts); mocks follow.
- summarizeCanvas() never lets a newline or a quote into a node line (the server
  validates review summaries line by line); test.
- test:browser:sitesmith-cleaned (Firefox + Chromium): a page published from a
  cleaned ordinary Sitesmith response requests nothing outside the allow-listed
  hosts (own assets, fonts, Font Awesome CDN, images.pexels.com, placehold.co)
  and has no inline handlers or javascript:/data: URL attributes; the same
  response uncleaned is the control (requests example.com). All requests are
  answered or aborted locally.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

craft/package.json

@@ -17,6 +17,7 @@
     "test:browser:colorpick": "node scripts/e2e-color-pick.mjs",
     "test:browser:colors": "node scripts/e2e-color-system.mjs",
     "test:browser:sitesmith": "node scripts/e2e-sitesmith.mjs",
+    "test:browser:sitesmith-cleaned": "node scripts/e2e-sitesmith-cleaned.mjs",
     "test:unit:watch": "vitest"
   },
   "dependencies": {

craft/scripts/e2e-sitesmith-cleaned-entry.ts

@@ -0,0 +1,31 @@
+// Bundled (esbuild) by scripts/e2e-sitesmith-cleaned.mjs: turns one ordinary Sitesmith response into a
+// published-style page, with or without the apply-time cleaner, through the real exporter.
+import { cleanAiResponse } from '../src/sitesmith/clean-ai-response';
+import { flattenTreeForCraft } from '../src/utils/craft-tree';
+import { exportToHtml } from '../src/utils/html-export';
+import { DEFAULT_SITE_DESIGN } from '../src/state/SiteDesignContext';
+import type { SitesmithResponse } from '../src/types/sitesmith';
+
+const n = (type: string, props: Record<string, unknown>, nodes?: any[]) => ({ type: { resolvedName: type }, props, ...(nodes ? { nodes } : {}) });
+
+export const RESPONSE: SitesmithResponse = {
+  type: 'replace', scope: 'page', message: 'Built a page.',
+  pages: [{ name: 'Home', tree: n('Container', { node_id: 'ROOT', tag: 'div' }, [
+    n('Navbar', { node_id: 'nav', logoType: 'text', logoText: 'Brand', logoUrl: '/', links: [{ text: 'Home', href: '/' }, { text: 'Old link', href: 'javascript:void(0)' }] }),
+    n('Heading', { node_id: 'h', text: 'Fresh bread every morning', level: 'h1' }),
+    n('ImageBlock', { node_id: 'i1', src: 'assets/hero.png', alt: 'Our bakery' }),
+    n('ImageBlock', { node_id: 'i2', src: 'https://example.com/photo.png', alt: 'Off-list host' }),
+    n('ImageBlock', { node_id: 'i3', src: 'https://placehold.co/600x400?text=Team', alt: 'Placeholder' }),
+    n('Container', { node_id: 'c', tag: 'div', style: { backgroundImage: 'url(https://example.com/bg.png)', padding: '24px' } }, [
+      n('ButtonLink', { node_id: 'b', text: 'Book a table', href: 'https://example.com/book' }),
+    ]),
+    n('VideoBlock', { node_id: 'v', videoUrl: 'https://example.com/clip.mp4' }),
+  ]) }],
+};
+
+export function render(clean: boolean): string {
+  const resp = clean ? cleanAiResponse(RESPONSE) : RESPONSE;
+  if (resp.type !== 'replace') throw new Error('unexpected');
+  const flat = flattenTreeForCraft(resp.pages[0].tree);
+  return exportToHtml(JSON.stringify(flat.nodes), { title: 'Home', includeFonts: true, design: DEFAULT_SITE_DESIGN }).html;
+}

craft/scripts/e2e-sitesmith-cleaned.mjs

@@ -0,0 +1,82 @@
+#!/usr/bin/env node
+/**
+ * Real-browser check (Firefox AND Chromium, headless) for the Sitesmith apply-time cleaner (G8): a page
+ * published from a CLEANED model response loads nothing outside the allow-listed hosts and carries no
+ * inline event handlers or javascript: URLs. The same response UNCLEANED is loaded as a control, to
+ * show the check can see an off-list request. Nothing leaves the machine: every request is answered
+ * or aborted by page.route (off-site requests are only recorded).
+ *   npm run test:browser:sitesmith-cleaned        Env: CHROMIUM_PATH / FIREFOX_PATH
+ */
+import { chromium, firefox } from '@playwright/test';
+import { build as esbuild } from 'esbuild';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
+const out = await esbuild({
+  entryPoints: [path.join(root, 'scripts/e2e-sitesmith-cleaned-entry.ts')],
+  bundle: true, write: false, format: 'esm', platform: 'node', jsx: 'automatic', logLevel: 'error',
+  define: { 'process.env.NODE_ENV': '"production"', __EDITOR_BUILD__: '"test"' },
+  banner: { js: "import { createRequire } from 'node:module'; const require = createRequire(import.meta.url);" },
+});
+const tmp = path.join(os.tmpdir(), `wsb-ss-cleaned-${process.pid}.mjs`);
+fs.writeFileSync(tmp, out.outputFiles[0].text);
+const ent = await import(tmp);
+fs.rmSync(tmp, { force: true });
+
+const SITE = 'http://site.test';
+// The published page's own head resources (fonts, Font Awesome) + the cleaner's image hosts.
+const ALLOWED_HOSTS = new Set(['site.test', 'fonts.googleapis.com', 'fonts.gstatic.com', 'cdnjs.cloudflare.com', 'images.pexels.com', 'placehold.co']);
+const PNG = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', 'base64');
+
+let pass = 0; let fail = 0;
+const ok = (c, m, d = '') => { if (c) { pass++; console.log('PASS ', m); } else { fail++; console.log('FAIL ', m, d); } };
+
+async function load(browser, html) {
+  const ctx = await browser.newContext();
+  const page = await ctx.newPage();
+  const hosts = new Set();
+  await page.route('**/*', async (route) => {
+    const u = new URL(route.request().url());
+    hosts.add(u.hostname);
+    if (u.origin === SITE) {
+      if (u.pathname === '/' || u.pathname === '/index.html') return route.fulfill({ status: 200, contentType: 'text/html', body: html });
+      if (u.pathname.startsWith('/assets/')) return route.fulfill({ status: 200, contentType: 'image/png', body: PNG });
+      return route.fulfill({ status: 404, body: '' });
+    }
+    return route.abort();      // never contact a real host
+  });
+  await page.goto(SITE + '/', { waitUntil: 'load' });
+  await page.waitForTimeout(300);
+  const dom = await page.evaluate(() => {
+    const handlers = [];
+    const jsUrls = [];
+    for (const el of document.querySelectorAll('*')) {
+      for (const a of el.attributes) {
+        if (/^on/i.test(a.name)) handlers.push(`${el.tagName}.${a.name}`);
+        if (['href', 'src', 'action', 'formaction', 'poster', 'data'].includes(a.name) && /^\s*(javascript|vbscript|data):/i.test(a.value)) jsUrls.push(`${el.tagName}.${a.name}`);
+      }
+    }
+    return { handlers, jsUrls, text: document.body.innerText };
+  });
+  await ctx.close();
+  return { hosts: [...hosts].sort(), dom };
+}
+
+for (const [name, launcher, exe] of [['chromium', chromium, process.env.CHROMIUM_PATH], ['firefox', firefox, process.env.FIREFOX_PATH]]) {
+  const browser = await launcher.launch({ executablePath: exe || undefined, ...(name === 'chromium' ? { args: ['--no-sandbox'] } : {}) });
+  const control = await load(browser, ent.render(false));
+  ok(control.hosts.includes('example.com'), `[${name}] control (uncleaned): the check sees the off-list request to example.com`, JSON.stringify(control.hosts));
+  const cleaned = await load(browser, ent.render(true));
+  const offList = cleaned.hosts.filter((h) => !ALLOWED_HOSTS.has(h));
+  ok(offList.length === 0, `[${name}] cleaned: no request outside the allow-listed hosts`, JSON.stringify(cleaned.hosts));
+  ok(cleaned.hosts.includes('site.test'), `[${name}] cleaned: the site's own asset was requested`, JSON.stringify(cleaned.hosts));
+  ok(cleaned.dom.handlers.length === 0, `[${name}] cleaned: no inline event handlers`, JSON.stringify(cleaned.dom.handlers));
+  ok(cleaned.dom.jsUrls.length === 0, `[${name}] cleaned: no javascript:/data: URLs in URL attributes`, JSON.stringify(cleaned.dom.jsUrls));
+  ok(/Fresh bread every morning/.test(cleaned.dom.text) && /Book a table/.test(cleaned.dom.text), `[${name}] cleaned: the ordinary content is all there`);
+  await browser.close();
+}
+console.log(fail ? `\nFAILED (${fail})` : `\nALL PASS (${pass})`);
+process.exit(fail ? 1 : 0);

craft/scripts/e2e-sitesmith-mock-server.mjs

@@ -288,6 +288,7 @@ export async function createMockServer(o) {
         s.refCalls.push({ action, body });
         refs = refs.filter((x) => x.id !== body.id);
         return json(res, 200, { ok: true });
+      case 'review_render_upload':
       case 'ref_image_upload': {
         const parts = parseMultipart(raw, ct);
         const fields = Object.fromEntries(parts.filter((p) => p.filename === null).map((p) => [p.name, p.data.toString('utf8')]));
@@ -298,7 +299,7 @@ export async function createMockServer(o) {
         const info = sniff(file.data);
         if (!info) return json(res, 415, { ok: false, error: 'unsupported_type', message: 'That file is not a picture Sitesmith can use. Use a JPEG, PNG, WebP or GIF.' });
         const id = crypto.randomBytes(16).toString('hex');
-        const meta = { id, kind: fields.kind === 'screenshot' ? 'screenshot' : 'mockup', name: file.filename || 'image', width: info.width, height: info.height, size: file.data.length, created_at: new Date().toISOString() };
+        const meta = { id, kind: action === 'review_render_upload' ? 'review_render' : fields.kind === 'screenshot' ? 'screenshot' : 'mockup', name: file.filename || 'image', width: info.width, height: info.height, size: file.data.length, created_at: new Date().toISOString() };
         images.set(id, { meta, bytes: file.data, mime: info.mime });
         s.uploads[s.uploads.length - 1].id = id;
         s.uploads[s.uploads.length - 1].mime = info.mime;

craft/scripts/e2e-sitesmith.mjs

@@ -320,8 +320,8 @@ scenario('8', 'self-review round trip (one round)', async ({ page, name, ok }) =
   ok('patch from the second job applied', true);
   ok('two start jobs', srv.starts.length === 2, String(srv.starts.length));
   const up = srv.uploads[0];
-  ok('render uploaded as a JPEG mockup named self-review-render.jpg, > 2000 bytes',
-    !!up && up.mime === 'image/jpeg' && up.kind === 'mockup' && up.name === 'self-review-render.jpg' && up.size > 2000, JSON.stringify(up));
+  ok('render uploaded through review_render_upload (kind review_render) as a JPEG named self-review-render.jpg, > 2000 bytes',
+    !!up && up.mime === 'image/jpeg' && up.kind === 'review_render' && up.name === 'self-review-render.jpg' && up.size > 2000, JSON.stringify(up));
   const s2 = srv.starts[1] ?? {};
   ok('second job message starts "Review the attached screenshot"', typeof s2.message === 'string' && s2.message.startsWith('Review the attached screenshot'), String(s2.message).slice(0, 60));
   ok('second job is intent "review" carrying the upload id in review_image_ids (not mockup_ids)', s2.intent === 'review' && Array.isArray(s2.review_image_ids) && s2.review_image_ids.includes(up?.id) && !s2.mockup_ids, JSON.stringify({ intent: s2.intent, review_image_ids: s2.review_image_ids, mockup_ids: s2.mockup_ids }));

craft/src/hooks/useSitesmith.ts

@@ -35,7 +35,7 @@ export interface SitesmithReference {
 
 export interface SitesmithRefImage {
   id: string;
-  kind: 'mockup' | 'screenshot';
+  kind: 'mockup' | 'screenshot' | 'review_render';
   name: string;
   width: number | null;
   height: number | null;
@@ -490,6 +490,13 @@ export function useSitesmith(siteId: number) {
     form.append('file', file);
     return call<{ image: SitesmithRefImage }>('ref_image_upload', { post: true, form });
   }, [call, siteId]);
+  /** The page render for ONE self-review job (backend kind 'review_render': never listed, deleted when the job ends). */
+  const uploadReviewRender = useCallback((file: File | Blob) => {
+    const form = new FormData();
+    form.append('site_id', String(siteId));
+    form.append('file', file);
+    return call<{ image: SitesmithRefImage }>('review_render_upload', { post: true, form });
+  }, [call, siteId]);
   const deleteRefImage = useCallback((id: string) =>
     call<{}>('ref_image_delete', { post: true, json: { site_id: siteId, id } }), [call, siteId]);
   const refImageUrl = useCallback((id: string): string =>
@@ -501,6 +508,6 @@ export function useSitesmith(siteId: number) {
     summary, messages, loading, error, send, refreshEntitlement, clearHistory,
     activeJob, resumeJob, cancel, capabilities, capabilitiesFailed,
     listRefs, addReference, updateReference, deleteReference,
-    uploadRefImage, deleteRefImage, refImageUrl, listAssets,
+    uploadRefImage, uploadReviewRender, deleteRefImage, refImageUrl, listAssets,
   };
 }

craft/src/panels/sitesmith/SitesmithModal.flows.test.tsx

@@ -52,7 +52,7 @@ function installFetch() {
     if (url.includes('action=history')) return json(net.history);
     if (url.includes('action=refs_list')) return json({ ok: true, references: net.refs, images: [] });
     if (url.includes('action=assets')) return json({ ok: true, assets: net.assets });
-    if (url.includes('action=ref_image_upload')) { net.uploads++; return net.uploadFail ? json({ ok: false, error: 'x', message: 'Upload failed' }, 500) : json({ ok: true, image: { id: 'f'.repeat(32), kind: 'mockup', name: 'self-review-render.jpg', width: 1280, height: 900, size: 5000 } }); }
+    if (url.includes('action=review_render_upload')) { net.uploads++; return net.uploadFail ? json({ ok: false, error: 'x', message: 'Upload failed' }, 500) : json({ ok: true, image: { id: 'f'.repeat(32), kind: 'mockup', name: 'self-review-render.jpg', width: 1280, height: 900, size: 5000 } }); }
     if (url.includes('action=ref_image_delete')) { net.deleted.push(JSON.parse(init.body).id); return json({ ok: true }); }
     return json({ ok: true, messages: [] });
   }));

craft/src/panels/sitesmith/SitesmithModal.tsx

@@ -198,7 +198,7 @@ export const SitesmithModal: React.FC<Props> = ({ onClose, target }) => {
         resolveAssetUrls,
         capture: (html, o) => captureHtml(html, o),
         upload: async (file) => {
-          const r = await ss.uploadRefImage(file, 'mockup');
+          const r = await ss.uploadReviewRender(file);
           return r.ok ? { ok: true, id: r.image.id } : { ok: false, message: r.message };
         },
         review: (msg, imageId) => send(msg, summarizeCanvas(query.getSerializedNodes()), undefined, {

craft/src/utils/canvas-summary.lines.test.ts

@@ -0,0 +1,16 @@
+import { describe, test, expect } from 'vitest';
+import { summarizeCanvas } from './canvas-summary';
+
+describe('summarizeCanvas: one line per node (self-review summaries are validated line by line)', () => {
+  test('newlines in text and quotes in names never split or break a line', () => {
+    const state: any = {
+      ROOT: { type: { resolvedName: 'Container' }, props: { tag: 'div' }, nodes: ['a'] },
+      a: { type: { resolvedName: 'TextBlock' }, props: { aiName: 'Say "hi"', node_id: 'ai-t1', text: 'Line one\nLine two\r\nthree' }, nodes: [] },
+    };
+    const lines = summarizeCanvas(state).split('\n');
+    expect(lines).toHaveLength(2);
+    expect(lines[1]).toBe(`  - TextBlock id=ai-t1 name="Say 'hi'" {text=Line one Line two three}`);
+    const lineRe = /^(?: {2})*- [A-Za-z][A-Za-z0-9]{0,40} id=([A-Za-z0-9_-]{1,64}) name="[^"\n]{0,120}" \{.{0,400}\}$/u;
+    for (const l of lines) expect(l).toMatch(lineRe);
+  });
+});

craft/src/utils/canvas-summary.ts

@@ -10,14 +10,17 @@ export function summarizeCanvas(state: SerializedNodes, maxChars = 6000): string
     const indent = '  '.repeat(depth);
     const type = typeof node.type === 'object' ? (node.type as any).resolvedName : String(node.type);
     const props = node.props || {};
-    const aiName = (props as any).aiName ?? '';
+    // One line per node, always: the server validates review summaries line by line
+    // (api/sitesmith.php sitesmith_review_summary_node_ids), so no newline or quote may leak in.
+    const flat = (t: string) => t.replace(/[\u0000-\u001f\u007f\u2028\u2029]+/g, ' ');
+    const aiName = flat(String((props as any).aiName ?? '')).replace(/"/g, "'").slice(0, 100);
     const nodeId = (props as any).node_id ?? id;
     const interesting: string[] = [];
     for (const [k, v] of Object.entries(props)) {
       // `html` (#46) is markup for the plain `text` mirror -- summarise text only.
       if (k === 'aiName' || k === 'node_id' || k === 'style' || k === 'html') continue;
       if (v == null) continue;
-      const repr = typeof v === 'string' ? v : JSON.stringify(v);
+      const repr = flat(typeof v === 'string' ? v : JSON.stringify(v));
       const truncated = repr.length > 60 ? repr.slice(0, 57) + '…' : repr;
       interesting.push(`${k}=${truncated}`);
       if (interesting.length >= 3) break;