sitesmith: a replacing header keeps links to existing pages; accurate chrome dialog

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

craft/src/panels/sitesmith/ScopeConfirmDialog.test.tsx

@@ -0,0 +1,29 @@
+import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
+import React from 'react';
+import { createRoot, Root } from 'react-dom/client';
+import { act } from 'react-dom/test-utils';
+import { ScopeConfirmDialog } from './ScopeConfirmDialog';
+
+let root: Root; let box: HTMLDivElement;
+beforeEach(() => { box = document.createElement('div'); document.body.appendChild(box); root = createRoot(box); });
+afterEach(() => { act(() => root.unmount()); box.remove(); });
+const render = (el: React.ReactElement) => act(async () => { root.render(el); });
+const text = () => box.textContent ?? '';
+
+describe('ScopeConfirmDialog chrome mode', () => {
+  test('no repair: other pages keep their content, the new header keeps links to every page; no false "not changed" claim', async () => {
+    await render(<ScopeConfirmDialog open mode="chrome" onConfirm={vi.fn()} onCancel={vi.fn()} />);
+    expect(text()).toContain('The content of your other pages is not changed');
+    expect(text()).toContain('keeps links to every page on your site');
+    expect(text()).not.toContain('your other pages are not changed');
+    expect(text()).not.toMatch(/left out/);
+  });
+  test('repair added links: says how many pages the header left out', async () => {
+    await render(<ScopeConfirmDialog open mode="chrome" addedLinks={['About', 'Services']} onConfirm={vi.fn()} onCancel={vi.fn()} />);
+    expect(text()).toContain("Sitesmith's header left out 2 of your pages (About, Services); links to them were added.");
+  });
+  test('singular', async () => {
+    await render(<ScopeConfirmDialog open mode="chrome" addedLinks={['About']} onConfirm={vi.fn()} onCancel={vi.fn()} />);
+    expect(text()).toContain("Sitesmith's header left out 1 of your pages (About); a link to it was added.");
+  });
+});

craft/src/panels/sitesmith/ScopeConfirmDialog.tsx

@@ -5,11 +5,13 @@ interface Props {
   pendingMessage?: string;
   /** 'site': replace every page + header + footer. 'chrome': the page is replaced either way; also use the new header/footer? */
   mode?: 'site' | 'chrome';
+  /** chrome mode: titles of existing pages the new header left out and the repair linked back in. */
+  addedLinks?: string[];
   onConfirm: () => void;
   onCancel: () => void;
 }
 
-export const ScopeConfirmDialog: React.FC<Props> = ({ open, pendingMessage, mode = 'site', onConfirm, onCancel }) => {
+export const ScopeConfirmDialog: React.FC<Props> = ({ open, pendingMessage, mode = 'site', addedLinks, onConfirm, onCancel }) => {
   if (!open) return null;
   const overlay: React.CSSProperties = { position: 'fixed', inset: 0, background: 'rgba(0,0,0,0.7)', zIndex: 10000, display: 'flex', alignItems: 'center', justifyContent: 'center' };
   const box:     React.CSSProperties = { background: '#1a1a2e', border: '1px solid #3f3f46', borderRadius: 10, padding: 22, maxWidth: 480, color: '#fff' };
@@ -24,7 +26,11 @@ export const ScopeConfirmDialog: React.FC<Props> = ({ open, pendingMessage, mode
             <p style={{ color: '#cbd5e1', fontSize: 14 }}>
               Your header and footer are still the starter ones ("MySite"). Sitesmith made a header and
               footer that match this page. They are shared, so every page will show them. This page is
-              replaced either way; your other pages are not changed.
+              replaced either way. The content of your other pages is not changed, and the new header
+              keeps links to every page on your site.
+              {addedLinks && addedLinks.length > 0 && (
+                <> Sitesmith's header left out {addedLinks.length} of your pages ({addedLinks.slice(0, 4).join(', ')}{addedLinks.length > 4 ? ', ...' : ''}); {addedLinks.length === 1 ? 'a link to it was' : 'links to them were'} added.</>
+              )}
             </p>
           </>
         ) : (

craft/src/sitesmith/header-repair.test.ts

@@ -0,0 +1,65 @@
+import { describe, test, expect } from 'vitest';
+import { repairHeaderLinks } from './header-repair';
+import type { PageData } from '../types';
+import type { SitesmithResponse } from '../types/sitesmith';
+
+const pg = (id: string, name: string, slug: string, nav?: any): PageData => ({ id, name, slug, craftState: null, nav });
+const PAGES = [pg('h', 'Home', 'index'), pg('a', 'About', 'about'), pg('s', 'Services', 'services')];
+
+const resp = (headerProps: Record<string, unknown>, extra: any[] = []): Extract<SitesmithResponse, { type: 'replace' }> => ({
+  type: 'replace', scope: 'page', message: 'm',
+  pages: [{ name: 'Home', tree: { type: { resolvedName: 'Container' }, props: {}, nodes: [] } }],
+  header: { tree: { type: { resolvedName: 'Container' }, props: {}, nodes: [{ type: { resolvedName: 'Navbar' }, props: { logoText: 'X', ...headerProps }, nodes: [] }, ...extra] } },
+});
+const nav = (r: any) => r.header.tree.nodes[0].props;
+
+describe('repairHeaderLinks', () => {
+  test('a nav that omits pages gets them back, followPages on, and reports which were missing', () => {
+    const r = repairHeaderLinks(resp({ links: [{ text: 'Home', href: '/' }] }), PAGES);
+    expect(r.missing).toEqual(['About', 'Services']);
+    const p = nav(r.resp);
+    expect(p.followPages).toBe(true);
+    expect(p.links.map((l: any) => l.href)).toEqual(['/', '/about', '/services']);
+    expect(p.links.every((l: any) => l.source === 'page')).toBe(true);
+  });
+  test('a nav that already has every page: no duplicates, nothing missing', () => {
+    const r = repairHeaderLinks(resp({ links: [{ text: 'Home', href: '/' }, { text: 'About', href: '/about' }, { text: 'Our services', href: '/services' }] }), PAGES);
+    expect(r.missing).toEqual([]);
+    const hrefs = nav(r.resp).links.map((l: any) => l.href);
+    expect(hrefs).toEqual(['/', '/about', '/services']);
+  });
+  test('manual in-page anchors and the CTA are kept', () => {
+    const r = repairHeaderLinks(resp({ links: [{ text: 'Pricing', href: '#pricing' }, { text: 'Home', href: '/' }, { text: 'Book now', href: '#book', isCta: true }] }), PAGES);
+    const links = nav(r.resp).links;
+    expect(links.find((l: any) => l.href === '#pricing')).toBeTruthy();
+    const cta = links.find((l: any) => l.isCta);
+    expect(cta).toMatchObject({ text: 'Book now', href: '#book' });
+    expect(links[links.length - 1].isCta).toBe(true);
+    expect(r.missing).toEqual(['About', 'Services']);
+  });
+  test('"/about.html" style hrefs count as the page (no duplicate)', () => {
+    const r = repairHeaderLinks(resp({ links: [{ text: 'About', href: '/about.html' }] }), PAGES);
+    expect(r.missing).toEqual(['Home', 'Services']);
+    expect(nav(r.resp).links.filter((l: any) => l.href === '/about')).toHaveLength(1);
+  });
+  test('a Menu in the header is repaired too; hidden pages are not required', () => {
+    const menu = { type: { resolvedName: 'Menu' }, props: { links: [] }, nodes: [] };
+    const pages = [...PAGES, pg('x', 'Secret', 'secret', { show: false })];
+    const r = repairHeaderLinks(resp({ links: [] }, [menu]), pages);
+    expect(r.missing).toEqual(['Home', 'About', 'Services']);
+    expect((r.resp as any).header.tree.nodes[1].props.followPages).toBe(true);
+    expect((r.resp as any).header.tree.nodes[1].props.links).toHaveLength(3);
+  });
+  test('no header, or no pages: unchanged', () => {
+    const noHeader = { ...resp({}), header: undefined } as any;
+    expect(repairHeaderLinks(noHeader, PAGES)).toEqual({ resp: noHeader, missing: [] });
+    const r = resp({ links: [] });
+    expect(repairHeaderLinks(r, []).resp).toBe(r);
+  });
+  test('does not mutate its input', () => {
+    const r = resp({ links: [] });
+    const snap = JSON.stringify(r);
+    repairHeaderLinks(r, PAGES);
+    expect(JSON.stringify(r)).toBe(snap);
+  });
+});

craft/src/sitesmith/header-repair.ts

@@ -0,0 +1,63 @@
+/**
+ * A new header must not cut the site off from its own pages (eval finding 4: Sitesmith's header listed
+ * three links and the other pages became unreachable). When a replace answer's header is going to
+ * replace the current one for a PAGE-scope request, every Navbar/Menu in it gets `followPages: true`
+ * and its links rebuilt with mergeLinks, which keeps the model's in-page anchors and CTA.
+ * Never used for a whole-site replace: there the pages are being replaced too, so the model's own
+ * links are the right ones.
+ */
+import type { PageData } from '../types';
+import type { SerializedTreeNode, SitesmithResponse } from '../types/sitesmith';
+import { adoptPageLinks, buildPageLinks, mergeLinks, pageHref, type NavLink } from '../utils/nav-links';
+
+const NAV = new Set(['Navbar', 'Menu']);
+
+/** "/about.html", "about", "./about/" -> "/about"; "index"/"index.html" -> "/". Anchors and external URLs are left alone. */
+function normHref(href: string): string {
+  const h = (href ?? '').trim();
+  if (!h || h.startsWith('#') || /^[a-z][a-z0-9+.-]*:/i.test(h) || h.startsWith('//')) return h;
+  let p = h.replace(/^\.\//, '').replace(/[?#].*$/, '').replace(/\.html?$/i, '').replace(/\/+$/, '');
+  if (!p.startsWith('/')) p = `/${p}`;
+  return p === '/index' ? '/' : p;
+}
+
+function flatten(links: NavLink[]): NavLink[] {
+  return links.flatMap((l) => [l, ...(Array.isArray(l.children) ? flatten(l.children) : [])]);
+}
+
+function repairNode(node: SerializedTreeNode, pages: PageData[], missing: Set<string>): SerializedTreeNode {
+  let out = node;
+  if (NAV.has(node.type?.resolvedName)) {
+    const raw: NavLink[] = Array.isArray(node.props?.links) ? (node.props.links as NavLink[]) : [];
+    const pageHrefs = new Map<string, string>(); // normalised href -> canonical href
+    pages.forEach((p, i) => pageHrefs.set(pageHref(p, i), pageHref(p, i)));
+    // Only manual links whose normalised href is a page get snapped to it (so adoptPageLinks recognises them).
+    const snapped = raw.map((l) => {
+      if (!l || l.isCta || l.source === 'page') return l;
+      const canon = pageHrefs.get(normHref(l.href));
+      return canon ? { ...l, href: canon } : l;
+    });
+    const have = new Set(flatten(snapped).map((l) => l.href));
+    // Pages the nav was meant to show (hidden pages never get a link).
+    const shown = flatten(buildPageLinks(pages));
+    for (const l of shown) if (!have.has(l.href)) missing.add(l.text);
+    const links = mergeLinks(adoptPageLinks(snapped, pages), pages);
+    out = { ...node, props: { ...node.props, followPages: true, links } };
+  }
+  if (Array.isArray(node.nodes) && node.nodes.length) {
+    const kids = node.nodes.map((k) => repairNode(k, pages, missing));
+    out = { ...out, nodes: kids };
+  }
+  return out;
+}
+
+/**
+ * Returns the answer with its header's navs repaired, and the titles of the existing pages the model's
+ * nav left out (before the repair). `resp` is returned untouched when there is nothing to repair.
+ */
+export function repairHeaderLinks(resp: SitesmithResponse, pages: PageData[]): { resp: SitesmithResponse; missing: string[] } {
+  if (resp.type !== 'replace' || !resp.header?.tree || !pages.length) return { resp, missing: [] };
+  const missing = new Set<string>();
+  const tree = repairNode(resp.header.tree, pages, missing);
+  return { resp: { ...resp, header: { ...resp.header, tree } }, missing: [...missing] };
+}

craft/src/sitesmith/replace-plan.test.ts

@@ -127,3 +127,29 @@ describe('missingUploadedImageNote (the brief requires an uploaded image when th
     expect(missingUploadedImageNote({ type: 'patch', message: 'm', ops: [] }, 3)).toBeNull();
   });
 });
+
+describe('planReplace: header link repair', () => {
+  const mk = (scope: 'page' | 'site'): any => ({
+    type: 'replace', scope, message: 'm',
+    pages: [{ name: 'Home', tree: { type: { resolvedName: 'Container' }, props: {}, nodes: [] } }],
+    header: { tree: { type: { resolvedName: 'Navbar' }, props: { links: [{ text: 'Home', href: '/' }] }, nodes: [] } },
+  });
+  const pages: any[] = [{ id: 'h', name: 'Home', slug: 'index', craftState: null }, { id: 'a', name: 'About', slug: 'about', craftState: null }];
+  test('page scope + chrome confirm: header nav repaired and the miss count reported', () => {
+    const plan = planReplace(mk('page'), { userText: 'redo this page', chromeDefault: true, pages });
+    expect(plan.confirm).toBe('chrome');
+    expect(plan.addedLinks).toEqual(['About']);
+    expect((plan.resp as any).header.tree.props.followPages).toBe(true);
+  });
+  test('site answer downgraded to a page is repaired as a page', () => {
+    const plan = planReplace(mk('site'), { userText: 'build my home page', chromeDefault: true, pages });
+    expect(plan.addedLinks).toEqual(['About']);
+  });
+  test('explicit whole-site replace: header left as the model wrote it', () => {
+    const r = mk('site');
+    const plan = planReplace(r, { userText: 'build the whole site', chromeDefault: true, pages });
+    expect(plan.confirm).toBe('site');
+    expect(plan.resp).toBe(r);
+    expect(plan.addedLinks).toBeUndefined();
+  });
+});

craft/src/sitesmith/replace-plan.ts

@@ -8,6 +8,8 @@
  * defaults ("MySite" + a "Get Started" button) and the owner confirms; otherwise they are kept.
  */
 import type { SitesmithResponse } from '../types/sitesmith';
+import type { PageData } from '../types';
+import { repairHeaderLinks } from './header-repair';
 
 const SITE_PATTERNS: RegExp[] = [
   /\b(?:whole|entire|full|complete)\s+(?:web\s*)?site\b/i,
@@ -71,9 +73,11 @@ export interface ReplacePlan {
   confirm: 'site' | 'chrome' | null;
   /** Shown to the owner when the plan changed what Sitesmith returned. */
   note?: string;
+  /** Titles of existing pages the new header left out and the repair linked back in (chrome confirm only). */
+  addedLinks?: string[];
 }
 
-export function planReplace(resp: SitesmithResponse, ctx: { userText: string | null | undefined; chromeDefault: boolean }): ReplacePlan {
+export function planReplace(resp: SitesmithResponse, ctx: { userText: string | null | undefined; chromeDefault: boolean; pages?: PageData[] }): ReplacePlan {
   if (resp.type !== 'replace' || resp.scope === 'section' || !resp.pages.length) return { resp, confirm: null };
   if (resp.scope === 'site' && explicitSiteRequest(ctx.userText)) return { resp, confirm: 'site' };
 
@@ -87,7 +91,15 @@ export function planReplace(resp: SitesmithResponse, ctx: { userText: string | n
   }
   const hasChrome = !!(page.header || page.footer);
   if (!hasChrome) return { resp: page, confirm: null, ...(notes.length ? { note: notes.join(' ') } : {}) };
-  if (ctx.chromeDefault) return { resp: page, confirm: 'chrome', ...(notes.length ? { note: notes.join(' ') } : {}) };
+  if (ctx.chromeDefault) {
+    // The new header replaces the current one for a page request: it must keep links to the existing pages.
+    const fixed = ctx.pages?.length ? repairHeaderLinks(page, ctx.pages) : { resp: page, missing: [] as string[] };
+    return {
+      resp: fixed.resp, confirm: 'chrome',
+      ...(notes.length ? { note: notes.join(' ') } : {}),
+      ...(fixed.missing.length ? { addedLinks: fixed.missing } : {}),
+    };
+  }
   const { header: _h, footer: _f, ...rest } = page;
   notes.push('Sitesmith also suggested a new header and footer, but yours have been edited, so it kept your header and footer.');
   return { resp: rest as SitesmithResponse, confirm: null, note: notes.join(' ') };