From 615a20e25bf3471a6d3e49de42d4cba6edbd5fce Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 1 Nov 2026 19:09:27 -0800 Subject: [PATCH] fix(sitesmith): footer page menus follow "Add these pages"; round 5 statuses; dialog wording Live run of 2026.11.9: - After "Add these pages" the footer's Explore menu listed only Home although the dialog said the header and footer link to every page. addPagesResponse() now also repairs the footer's PAGE menus (repairFooterLinks: a Navbar/Menu that already links a page of the site gets followPages + every shown page); a legal/social footer menu is left alone. - Dialog: "keeps your other 1 page" -> "keeps your other page (name)" / "keeps your other N pages (names)"; "with menus that link to every page you will have". - New server statuses PROVIDER_BUDGET, PROVIDER_AUTH (the provider refused this server's key) and SITE_OVER_LIMIT (whole-site pre-flight) with specific messages; the server text is preferred (fixed wording + derived reason). Co-Authored-By: Claude Opus 5.5 --- .../sitesmith/ScopeConfirmDialog.test.tsx | 11 +++++ .../panels/sitesmith/ScopeConfirmDialog.tsx | 5 ++- .../panels/sitesmith/statusMessages.test.ts | 22 ++++++++++ craft/src/panels/sitesmith/statusMessages.ts | 6 +++ craft/src/sitesmith/header-repair.ts | 25 +++++++++-- craft/src/sitesmith/replace-plan.test.ts | 42 +++++++++++++++++++ craft/src/sitesmith/replace-plan.ts | 9 ++-- craft/src/types/sitesmith.ts | 6 ++- 8 files changed, 116 insertions(+), 10 deletions(-) diff --git a/craft/src/panels/sitesmith/ScopeConfirmDialog.test.tsx b/craft/src/panels/sitesmith/ScopeConfirmDialog.test.tsx index 39bad5b..0352e6b 100644 --- a/craft/src/panels/sitesmith/ScopeConfirmDialog.test.tsx +++ b/craft/src/panels/sitesmith/ScopeConfirmDialog.test.tsx @@ -55,6 +55,17 @@ describe('ScopeConfirmDialog site modes', () => { expect(text()).toContain('This page was not built: About.'); expect(btns()).toEqual(['Cancel', 'Add these pages']); }); + test('r5: ONE kept page reads "keeps your other page (name)", never "your other 1 page"; the menus claim is the truthful one', async () => { + await render(); + expect(text()).toContain('keeps your other page (MG TEST 2026.11.5 (delete me))'); + expect(text()).not.toMatch(/other 1 page/); + expect(text()).toContain('uses the new header and footer, with menus that link to every page you will have.'); + expect(text()).not.toContain('(they link to every page you will have)'); + }); + test('r5: several kept pages are counted and named', async () => { + await render(); + expect(text()).toContain('keeps your other 2 pages (MG TEST, Team)'); + }); test('complete site build: the replace confirm names the pages it deletes', async () => { await render(); expect(text()).toContain('Replace your entire site?'); diff --git a/craft/src/panels/sitesmith/ScopeConfirmDialog.tsx b/craft/src/panels/sitesmith/ScopeConfirmDialog.tsx index a80736a..aa70d6a 100644 --- a/craft/src/panels/sitesmith/ScopeConfirmDialog.tsx +++ b/craft/src/panels/sitesmith/ScopeConfirmDialog.tsx @@ -65,7 +65,8 @@ export const ScopeConfirmDialog: React.FC = ({ open, pendingMessage, mode const addText = [ replaced.length ? `replaces your ${list(replaced)} ${replaced.length === 1 ? 'page' : 'pages'}` : '', impact.added.length ? `adds ${list(impact.added)}` : '', - kept.length ? `keeps your other ${plural(kept.length, 'page')}` : '', + // round 5: "keeps your other 1 page" -> "keeps your other page (MG TEST)" + kept.length === 1 ? `keeps your other page (${list(kept)})` : kept.length ? `keeps your other ${kept.length} pages (${list(kept)})` : '', ].filter(Boolean).join(', '); body = ( <> @@ -74,7 +75,7 @@ export const ScopeConfirmDialog: React.FC = ({ open, pendingMessage, mode {missing.length === 1 ? 'This page was not built' : 'These pages were not built'}: {list(missing)}. You can ask for {missing.length === 1 ? 'it' : 'each one'} on its own afterwards.

- Add these pages {addText || 'adds the new pages'}, and uses the new header and footer (they link to every page you will have). + Add these pages {addText || 'adds the new pages'}, and uses the new header and footer, with menus that link to every page you will have.

{kept.length > 0 && (

diff --git a/craft/src/panels/sitesmith/statusMessages.test.ts b/craft/src/panels/sitesmith/statusMessages.test.ts index cbb317f..206b7f7 100644 --- a/craft/src/panels/sitesmith/statusMessages.test.ts +++ b/craft/src/panels/sitesmith/statusMessages.test.ts @@ -46,3 +46,25 @@ describe('Sitesmith failure messages (G6)', () => { expect(failureMessage({ ok: false, status: 'AI_ERROR', message: 'x' })).toBe(STATUS_MESSAGES.AI_ERROR.text); }); }); + +// ---- round 5: the provider refused this server's key / a whole site was refused before it started +describe('round 5 statuses', () => { + test('PROVIDER_BUDGET, PROVIDER_AUTH and SITE_OVER_LIMIT are known server statuses (not the generic ERROR)', () => { + for (const st of ['PROVIDER_BUDGET', 'PROVIDER_AUTH', 'SITE_OVER_LIMIT']) { + expect(isSitesmithStatus(st), st).toBe(true); + expect(failureMessage({ ok: false, status: st as SitesmithStatus, message: '' })).not.toBe(STATUS_MESSAGES.ERROR.text); + } + }); + test('without server text they say who can fix it, and never suggest retrying the same thing', () => { + expect(failureMessage({ ok: false, status: 'PROVIDER_BUDGET', message: '' })).toMatch(/budget.*administrator/i); + expect(failureMessage({ ok: false, status: 'PROVIDER_AUTH', message: '' })).toMatch(/key.*administrator/i); + expect(failureMessage({ ok: false, status: 'SITE_OVER_LIMIT', message: '' })).toMatch(/one page at a time/); + for (const st of ['PROVIDER_BUDGET', 'PROVIDER_AUTH'] as SitesmithStatus[]) expect(failureMessage({ ok: false, status: st, message: '' })).not.toMatch(/try again/i); + }); + test('the server text (fixed wording + derived reason, never gateway text) is shown when present', () => { + const m = "The AI provider's budget for this server's Sitesmith key is used up, so Sitesmith cannot make AI calls right now. Please ask your administrator to raise the budget. Reason: budget or quota exceeded (HTTP 422)."; + expect(failureMessage({ ok: false, status: 'PROVIDER_BUDGET', message: m })).toBe(m); + const u = "Building a whole website takes up to 5 AI calls (a plan, then one per page), and that could go over what is left of this server's daily Sitesmith limit, so Sitesmith did not start."; + expect(failureMessage({ ok: false, status: 'SITE_OVER_LIMIT', message: u })).toBe(u); + }); +}); diff --git a/craft/src/panels/sitesmith/statusMessages.ts b/craft/src/panels/sitesmith/statusMessages.ts index f4c2922..fdfb33b 100644 --- a/craft/src/panels/sitesmith/statusMessages.ts +++ b/craft/src/panels/sitesmith/statusMessages.ts @@ -27,6 +27,12 @@ export const STATUS_MESSAGES: Record [l, ...(Array.isArray(l.children) ? flatten(l.children) : [])]); } -function repairNode(node: SerializedTreeNode, pages: PageData[], missing: Set): SerializedTreeNode { +/** Does this nav already link at least one of the site's pages (the footer's "Explore" menu, not a "Legal" one)? */ +function linksAPage(node: SerializedTreeNode, pages: PageData[]): boolean { + const hrefs = new Set(pages.map((p, i) => pageHref(p, i))); + const raw: NavLink[] = Array.isArray(node.props?.links) ? (node.props.links as NavLink[]) : []; + return flatten(raw).some((l) => !!l && (l.source === 'page' || hrefs.has(normHref(l.href)))); +} + +function repairNode(node: SerializedTreeNode, pages: PageData[], missing: Set, onlyPageMenus = false): SerializedTreeNode { let out = node; - if (NAV.has(node.type?.resolvedName)) { + if (NAV.has(node.type?.resolvedName) && (!onlyPageMenus || linksAPage(node, pages))) { const raw: NavLink[] = Array.isArray(node.props?.links) ? (node.props.links as NavLink[]) : []; const pageHrefs = new Map(); // normalised href -> canonical href pages.forEach((p, i) => pageHrefs.set(pageHref(p, i), pageHref(p, i))); @@ -45,7 +52,7 @@ function repairNode(node: SerializedTreeNode, pages: PageData[], missing: Set repairNode(k, pages, missing)); + const kids = node.nodes.map((k) => repairNode(k, pages, missing, onlyPageMenus)); out = { ...out, nodes: kids }; } return out; @@ -61,3 +68,15 @@ export function repairHeaderLinks(resp: SitesmithResponse, pages: PageData[]): { const tree = repairNode(resp.header.tree, pages, missing); return { resp: { ...resp, header: { ...resp.header, tree } }, missing: [...missing] }; } + +/** + * Round 5 (live run of 2026.11.9): after "Add these pages" the footer's "Explore" menu listed only Home, although + * the dialog promised menus linking every page the owner will have. The footer's PAGE menus -- a Navbar/Menu that + * already links at least one page of the site -- are repaired the same way as the header (followPages + every + * shown page); a footer menu of other links only (legal, social, anchors) is left alone. + */ +export function repairFooterLinks(resp: SitesmithResponse, pages: PageData[]): SitesmithResponse { + if (resp.type !== 'replace' || !resp.footer?.tree || !pages.length) return resp; + const tree = repairNode(resp.footer.tree, pages, new Set(), true); + return { ...resp, footer: { ...resp.footer, tree } }; +} diff --git a/craft/src/sitesmith/replace-plan.test.ts b/craft/src/sitesmith/replace-plan.test.ts index 95c72d1..873c2b0 100644 --- a/craft/src/sitesmith/replace-plan.test.ts +++ b/craft/src/sitesmith/replace-plan.test.ts @@ -208,3 +208,45 @@ describe('round 4: partial whole-site builds', () => { expect(out.pages).toEqual((siteResp(["Home"]) as any).pages); }); }); + +// ---- round 5 item 3 (live run of 2026.11.9): after "Add these pages" the footer's Explore column listed only Home, +// although the dialog promised menus linking every page the owner will have. +describe('round 5: "Add these pages" repairs the footer page menus too', () => { + const tree = { type: { resolvedName: 'Container' }, props: {}, nodes: [] } as any; + const existing: any[] = [ + { id: 'home', name: 'Home', slug: 'index', craftState: 'H' }, + { id: 'p2', name: 'MG TEST 2026.11.5 (delete me)', slug: 'mobile-guard-test', craftState: 'M' }, + ]; + // the case-2 footer: brand column + an "Explore" Menu (pruned by the server to the built Home) + a legal menu + const footer = { type: { resolvedName: 'Container' }, props: { tag: 'footer' }, nodes: [ + { type: { resolvedName: 'ColumnLayout' }, props: { columns: 3 }, nodes: [ + { type: { resolvedName: 'Container' }, props: {}, nodes: [{ type: { resolvedName: 'Logo' }, props: { text: 'OLS Digital', href: '/' }, nodes: [] }] }, + { type: { resolvedName: 'Container' }, props: {}, nodes: [{ type: { resolvedName: 'Menu' }, props: { links: [{ text: 'Home', href: '/' }] }, nodes: [] }] }, + { type: { resolvedName: 'Container' }, props: {}, nodes: [{ type: { resolvedName: 'Menu' }, props: { links: [{ text: 'Privacy', href: '#privacy' }, { text: 'Mail', href: 'mailto:a@b.c' }] }, nodes: [] }] }, + ] }, + ] } as any; + const resp: SitesmithResponse = { type: 'replace', scope: 'site', message: 'Built 1 page: Home.', pages: [{ name: 'Home', tree }], + header: { tree: { type: { resolvedName: 'Navbar' }, props: { links: [{ text: 'Home', href: '/' }] }, nodes: [] } as any }, footer: { tree: footer } }; + const cols = (r: any) => r.footer.tree.nodes[0].nodes; + + test('the footer Explore menu links the kept page as well as the built one (and follows pages from now on)', async () => { + const { addPagesResponse } = await import('./replace-plan'); + const out = addPagesResponse(resp, existing) as any; + const explore = cols(out)[1].nodes[0].props; + expect(explore.links.map((l: any) => l.href)).toEqual(['/', '/mobile-guard-test']); + expect(explore.followPages).toBe(true); + expect(out.header.tree.props.links.map((l: any) => l.href)).toEqual(['/', '/mobile-guard-test']); + }); + test('a footer menu of other links only (legal, mail) is left exactly as it was; so is the logo', async () => { + const { addPagesResponse } = await import('./replace-plan'); + const out = addPagesResponse(resp, existing) as any; + expect(cols(out)[2]).toEqual(footer.nodes[0].nodes[2]); + expect(cols(out)[0]).toEqual(footer.nodes[0].nodes[0]); + }); + test('the input is not mutated', async () => { + const { addPagesResponse } = await import('./replace-plan'); + const snap = JSON.stringify(resp); + addPagesResponse(resp, existing); + expect(JSON.stringify(resp)).toBe(snap); + }); +}); diff --git a/craft/src/sitesmith/replace-plan.ts b/craft/src/sitesmith/replace-plan.ts index 74cc505..b134c0f 100644 --- a/craft/src/sitesmith/replace-plan.ts +++ b/craft/src/sitesmith/replace-plan.ts @@ -9,7 +9,7 @@ */ import type { SitesmithResponse } from '../types/sitesmith'; import type { PageData } from '../types'; -import { repairHeaderLinks } from './header-repair'; +import { repairHeaderLinks, repairFooterLinks } from './header-repair'; import { mergeAiPages } from '../state/PageContext'; const SITE_PATTERNS: RegExp[] = [ @@ -136,13 +136,14 @@ export function planReplace(resp: SitesmithResponse, ctx: { userText: string | n } /** - * "Add these pages": the whole-site answer applied without deleting pages. The new header must then link - * the kept pages as well as the built ones (repairHeaderLinks against the merged page list). + * "Add these pages": the whole-site answer applied without deleting pages. The new header AND the footer's page + * menus must then link the kept pages as well as the built ones (repairHeaderLinks / repairFooterLinks against the + * merged page list) -- the dialog says so (round 5: the footer used to keep only the built pages). */ export function addPagesResponse(resp: SitesmithResponse, pages: PageData[]): SitesmithResponse { if (resp.type !== 'replace') return resp; const merged = mergeAiPages(pages, resp.pages.map((p) => ({ name: p.name, craftState: null })), mintPreviewId()).pages; - return repairHeaderLinks(resp, merged).resp; + return repairFooterLinks(repairHeaderLinks(resp, merged).resp, merged); } /** The same page answer without the header/footer (the owner chose to keep them). */ diff --git a/craft/src/types/sitesmith.ts b/craft/src/types/sitesmith.ts index 543ce38..177478e 100644 --- a/craft/src/types/sitesmith.ts +++ b/craft/src/types/sitesmith.ts @@ -7,7 +7,10 @@ export type SitesmithServerStatus = | 'USER_KILLSWITCH' | 'SERVER_KILLSWITCH' | 'RATE_LIMITED' | 'BLOCKED' | 'AI_ERROR' | 'AI_INVALID' | 'BAD_ATTACHMENT' | 'IMAGES_BUSY' | 'INTERRUPTED' | 'CANCELLED' - | 'JOB_IN_PROGRESS' | 'TOO_MANY_JOBS' | 'SERVER_BUSY' | 'RESPONSE_TOO_LONG'; + | 'JOB_IN_PROGRESS' | 'TOO_MANY_JOBS' | 'SERVER_BUSY' | 'RESPONSE_TOO_LONG' + // round 5: the AI provider refused this server's key (budget/quota used up | key not accepted); a whole-site + // build that could go over what is left of a daily limit is refused before it starts + | 'PROVIDER_BUDGET' | 'PROVIDER_AUTH' | 'SITE_OVER_LIMIT'; /** Statuses the editor's transport (hooks/useSitesmith.ts) produces itself. */ export type SitesmithClientStatus = @@ -20,6 +23,7 @@ export const SITESMITH_SERVER_STATUSES: readonly SitesmithServerStatus[] = [ 'OK_BONUS', 'OK_MONTHLY', 'DISABLED', 'CAP_REACHED', 'USER_KILLSWITCH', 'SERVER_KILLSWITCH', 'RATE_LIMITED', 'BLOCKED', 'AI_ERROR', 'AI_INVALID', 'BAD_ATTACHMENT', 'IMAGES_BUSY', 'INTERRUPTED', 'CANCELLED', 'JOB_IN_PROGRESS', 'TOO_MANY_JOBS', 'SERVER_BUSY', 'RESPONSE_TOO_LONG', + 'PROVIDER_BUDGET', 'PROVIDER_AUTH', 'SITE_OVER_LIMIT', ]; export const SITESMITH_CLIENT_STATUSES: readonly SitesmithClientStatus[] = [ 'NETWORK', 'TIMEOUT', 'ABORTED', 'LOST_CONNECTION', 'AUTH_REQUIRED', 'FORBIDDEN', 'TOO_LARGE',