fix(visual): harden sitemap crawler and make it opt-in (SEL-453)

frontend/playwright.visual.config.ts

@@ -1,10 +1,12 @@
 /**
  * Report-only visual regression + page monitoring config.
  *
- * Read-only by design: every project in this file visits pages with GET only,
- * except the two conditional projects below, which do not exist unless their
- * env flag is set (so nothing ever shows as "skipped"):
+ * Read-only by design: every project in this file visits pages with GET only.
+ * Conditional projects below do not exist unless their env flag is set
+ * (so nothing ever shows as "skipped"):
  *
+ * - crawl-desktop / crawl-mobile: sitemap crawler (crawl.spec.ts).
+ *   Opt-in via VISUAL_CRAWL=1. No workflow sets VISUAL_CRAWL.
  * - preview-forms: blank-submit validation screenshots on /login + /signup.
  *   Only on preview deploys, via VISUAL_ALLOW_FORM_SUBMIT=1. Never fills
  *   credentials, never creates an account.
@@ -21,6 +23,7 @@ import { defineConfig } from '@playwright/test';
 
 const allowFormSubmit = process.env.VISUAL_ALLOW_FORM_SUBMIT === '1';
 const allowSignedIn = process.env.VISUAL_AUTH === '1';
+const allowCrawl = process.env.VISUAL_CRAWL === '1';
 
 export default defineConfig({
     testDir: './tests/visual',
@@ -90,22 +93,26 @@ export default defineConfig({
             name: 'smoke',
             testMatch: ['smoke.spec.ts'],
         },
-        {
-            name: 'crawl-desktop',
-            testMatch: ['crawl.spec.ts'],
-            use: { viewport: { width: 1440, height: 900 }, deviceScaleFactor: 1 },
-        },
-        {
-            name: 'crawl-mobile',
-            testMatch: ['crawl.spec.ts'],
-            use: {
-                viewport: { width: 390, height: 844 },
-                isMobile: true,
-                hasTouch: true,
-                deviceScaleFactor: 1,
-            },
-        },
         /* Conditional projects: absent unless the env flag is set. */
+        ...(allowCrawl
+            ? [
+                  {
+                      name: 'crawl-desktop',
+                      testMatch: ['crawl.spec.ts'],
+                      use: { viewport: { width: 1440, height: 900 }, deviceScaleFactor: 1 },
+                  },
+                  {
+                      name: 'crawl-mobile',
+                      testMatch: ['crawl.spec.ts'],
+                      use: {
+                          viewport: { width: 390, height: 844 },
+                          isMobile: true,
+                          hasTouch: true,
+                          deviceScaleFactor: 1,
+                      },
+                  },
+              ]
+            : []),
         ...(allowFormSubmit
             ? [
                   {

frontend/tests/visual/README.md

@@ -38,8 +38,8 @@ VISUAL_BASE_URL=https://regengine.co npx playwright test --config=playwright.vis
 # One project
 VISUAL_BASE_URL=https://regengine.co npx playwright test --config=playwright.visual.config.ts --project=smoke
 
-# Sitemap crawler at both widths
-VISUAL_BASE_URL=https://regengine.co npx playwright test --config=playwright.visual.config.ts --project=crawl-desktop --project=crawl-mobile
+# Sitemap crawler at both widths (opt-in via VISUAL_CRAWL=1)
+VISUAL_BASE_URL=https://regengine.co VISUAL_CRAWL=1 npx playwright test --config=playwright.visual.config.ts --project=crawl-desktop --project=crawl-mobile
 
 # View the last HTML report
 npx playwright show-report playwright-visual-report
@@ -52,6 +52,7 @@ npx playwright show-report playwright-visual-report
 | `VISUAL_BASE_URL` | Base URL under test. Default `https://regengine.co` |
 | `VISUAL_ALLOW_FORM_SUBMIT` | `1` enables the `preview-forms` project (blank submits). Set only by `visual-preview.yml` |
 | `VISUAL_AUTH` | `1` enables the `signed-in` project. Set by hand for manual runs; no workflow sets it |
+| `VISUAL_CRAWL` | `1` enables the `crawl-desktop` / `crawl-mobile` projects. Set by hand for manual runs; no workflow sets it |
 | `VISUAL_USER_EMAIL` | Expected session email for the signed-in project (asserted, not used to log in) |
 | `VISUAL_SESSION_TOKEN` | Opaque session cookie value for the signed-in project, provisioned out-of-band |
 | `VISUAL_UPDATE_BOXES` | `1` records header/nav boxes to `test-results-visual/layout-boxes.candidate.json` (JSONL) instead of asserting |
@@ -66,8 +67,8 @@ npx playwright show-report playwright-visual-report
 | `priority-extras` | priority extras | mixed (set per test) |
 | `content` | content rules | default |
 | `smoke` | smoke | default |
-| `crawl-desktop` | sitemap crawl | 1440×900 |
-| `crawl-mobile` | sitemap crawl | 390×844, touch |
+| `crawl-desktop` | sitemap crawl | 1440×900, only if `VISUAL_CRAWL=1` |
+| `crawl-mobile` | sitemap crawl | 390×844, touch, only if `VISUAL_CRAWL=1` |
 | `preview-forms` | blank submits | default, only if `VISUAL_ALLOW_FORM_SUBMIT=1` |
 | `signed-in` | signed-in reads | default, only if `VISUAL_AUTH=1` |
 

frontend/tests/visual/crawl.spec.ts

@@ -41,15 +41,70 @@ interface PageFinding {
 }
 
 function baseOriginOf(page: Page): string {
-    return new URL(test.info().project.use.baseURL ?? page.url()).origin;
+    const raw =
+        test.info().project.use.baseURL ??
+        process.env.VISUAL_BASE_URL ??
+        page.url();
+    let origin: string;
+    try {
+        origin = new URL(raw).origin;
+    } catch {
+        throw new Error(`Cannot determine crawl base origin from "${raw}": invalid URL`);
+    }
+    if (origin === 'null' || origin === '') {
+        const fallback = process.env.VISUAL_BASE_URL ?? 'https://regengine.co';
+        try {
+            const fallbackOrigin = new URL(fallback).origin;
+            if (fallbackOrigin !== 'null' && fallbackOrigin !== '') {
+                return fallbackOrigin;
+            }
+        } catch {
+            // Fall through to the clear error below.
+        }
+        throw new Error(
+            `Cannot determine crawl base origin from "${raw}": got opaque origin 'null'. ` +
+                'Set VISUAL_BASE_URL to a valid http(s) URL.',
+        );
+    }
+    return origin;
+}
+
+/** Rewrite a sitemap URL to the base origin so preview runs crawl the preview. */
+function rewriteToBaseOrigin(loc: string, baseOrigin: string): string {
+    try {
+        const url = new URL(loc, baseOrigin);
+        const base = new URL(baseOrigin);
+        url.protocol = base.protocol;
+        url.host = base.host;
+        return url.toString();
+    } catch {
+        return loc;
+    }
+}
+
+/** Decode XML entities in a <loc> value; &amp; is decoded last. */
+function decodeXmlEntities(value: string): string {
+    return value
+        .replace(/&#x([0-9A-Fa-f]+);/g, (_m: string, hex: string) =>
+            String.fromCodePoint(parseInt(hex, 16)),
+        )
+        .replace(/&#([0-9]+);/g, (_m: string, dec: string) =>
+            String.fromCodePoint(parseInt(dec, 10)),
+        )
+        .replace(/&lt;/g, '<')
+        .replace(/&gt;/g, '>')
+        .replace(/&quot;/g, '"')
+        .replace(/&apos;/g, "'")
+        .replace(/&amp;/g, '&');
 }
 
 /** Extract <loc> values from a sitemap or sitemap-index document. */
 function parseLocs(xml: string): string[] {
     const locs: string[] = [];
     const re = /<loc>\s*([^<]+?)\s*<\/loc>/gi;
     let match: RegExpExecArray | null;
-    while ((match = re.exec(xml)) !== null) locs.push(match[1].trim());
+    while ((match = re.exec(xml)) !== null)
+        locs.push(decodeXmlEntities(match[1].trim()));
     return locs;
 }
 
@@ -71,11 +126,13 @@ async function fetchSitemapPaths(page: Page, baseOrigin: string): Promise<string
     if (locs.length > 0 && locs.every((loc) => isSitemapUrl(loc))) {
         const childPaths: string[] = [];
         for (const loc of locs) {
-            const response = await page.request.get(loc, {
+            const childUrl = rewriteToBaseOrigin(loc, baseOrigin);
+            const response = await page.request.get(childUrl, {
                 timeout: LINK_TIMEOUT_MS,
                 maxRedirects: 5,
             });
-            if (!response.ok()) continue;
+            if (!response.ok())
+                throw new Error(`child sitemap GET failed: ${childUrl} status ${response.status()}`);
             childPaths.push(...parseLocs(await response.text()));
         }
         locs = childPaths;
@@ -92,6 +149,7 @@ async function fetchSitemapPaths(page: Page, baseOrigin: string): Promise<string
         // Rewrite to the base URL's origin so preview runs crawl the preview.
         paths.add(url.pathname + (url.search || ''));
     }
+    if (paths.size === 0) throw new Error('sitemap has no usable page URLs');
     return [...paths].sort();
 }
 
@@ -196,6 +254,7 @@ test('crawl sitemap pages for broken links, dead ends, and errors', async ({
         page.on('pageerror', onPageError);
         page.on('console', onConsole);
         let status = 0;
+        let navigationFailed = false;
         try {
             const response = await page.goto(crawlPath, {
                 waitUntil: 'domcontentloaded',
@@ -206,41 +265,57 @@ test('crawl sitemap pages for broken links, dead ends, and errors', async ({
                 .waitForLoadState('networkidle', { timeout: 15_000 })
                 .catch(() => undefined);
         } catch (error) {
+            navigationFailed = true;
             pageErrors.push(`navigation failed: ${String(error).slice(0, 300)}`);
         } finally {
             page.off('pageerror', onPageError);
             page.off('console', onConsole);
         }
 
         const pageUrl = page.url();
-        const hrefs = await page.locator('a[href]').evaluateAll((anchors) =>
-            anchors.map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? ''),
-        );
-        const scopedHrefs = await page
-            .locator('main a[href], nav a[href], footer a[href]')
-            .evaluateAll((anchors) =>
+        // Scope links per page: a failed load keeps the previous page's DOM,
+        // so it must record no links rather than crediting them to this path.
+        let scopedSameOrigin = 0;
+        if (!navigationFailed) {
+            const hrefs = await page.locator('a[href]').evaluateAll((anchors) =>
                 anchors.map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? ''),
             );
-        let scopedSameOrigin = 0;
-        for (const href of scopedHrefs) {
-            if (normalizeLink(href, pageUrl, baseOrigin)) scopedSameOrigin += 1;
-        }
-        for (const href of hrefs) {
-            const target = normalizeLink(href, pageUrl, baseOrigin);
-            if (target && !linkSources.has(target)) linkSources.set(target, crawlPath);
+            const scopedHrefs = await page
+                .locator('main a[href], nav a[href], footer a[href]')
+                .evaluateAll((anchors) =>
+                    anchors.map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? ''),
+                );
+            const normalizedPageUrl = normalizeLink(pageUrl, pageUrl, baseOrigin);
+            for (const href of scopedHrefs) {
+                const scopedTarget = normalizeLink(href, pageUrl, baseOrigin);
+                if (scopedTarget && scopedTarget !== normalizedPageUrl)
+                    scopedSameOrigin += 1;
+            }
+            for (const href of hrefs) {
+                const target = normalizeLink(href, pageUrl, baseOrigin);
+                if (target && !linkSources.has(target)) linkSources.set(target, crawlPath);
+            }
         }
 
         let deadEndReason: string | null = null;
-        if (status >= 400) {
+        // A failed navigation leaves stale DOM behind and is already recorded
+        // in pageErrors, so skip dead-end classification entirely.
+        if (!navigationFailed && status >= 400) {
             deadEndReason = `HTTP ${status}`;
-        } else {
+        } else if (!navigationFailed) {
             let finalPath = '';
             try {
                 finalPath = new URL(pageUrl).pathname;
             } catch {
                 finalPath = '';
             }
-            const h1 = ((await page.locator('h1').first().textContent().catch(() => '')) ?? '').trim();
+            const h1 = (
+                (await page
+                    .locator('h1')
+                    .first()
+                    .textContent({ timeout: 5_000 })
+                    .catch(() => '')) ?? ''
+            ).trim();
             if (finalPath === '/404' || finalPath === '/not-found' || h1 === '404') {
                 deadEndReason = `ends on 404 page (${finalPath || pageUrl})`;
             } else if (scopedSameOrigin === 0) {