fix: scope global-* to the app root and stop // in a url blanking a line

Two behaviour defects and four stale claims.

The widened module set added global-error and global-not-found, but
router.js registers both only at the app root (dir === '.'), so a nested
one is never routed and warning on it is advice about dead code, the same
class the _private skip exists to avoid.

The // comment blanking was a flat regex guarded only against https://.
It fired on a protocol-relative href="//cdn/x.css" and blanked the rest
of the line, so a layout mixing a CDN sheet with a local one went
silently inert: exactly the shape this check targets. Comment blanking is
a quote-aware walk now, because the two comment families live in opposite
contexts. An HTML comment must be blanked INSIDE the template literal
holding the tags, a JS comment only OUTSIDE a string.

The recorded rationale for skipping a query or .. href was also wrong. It
claimed asset() could never clear the warning, but this check reads the
SOURCE shape, so a wrapped href is an unquoted hole and does clear it.
The real reason is that the wrap is a runtime no-op there, so it would
clear the warning without improving the caching.

Remaining edits are the scope wording that widening left stale: the
user-facing pass message, three in-file contract lines, the docs-site
configuration page, and a built-ins sentence that still said this was a
convention rather than something a tool checks.

Author: vivek7405

.agents/skills/webjs/references/built-ins.md

@@ -91,7 +91,7 @@ import { html, asset } from '@webjsdev/core';
 html`<link rel="stylesheet" href=${asset('/public/app.css')}>`
 ```
 
-That emits `/public/app.css?v=<hash>` in production and gets the immutable year; the same url un-marked gets a ~1h cap and can serve stale bytes from a CDN after a deploy until something purges it. `asset()` resolves on the server; the browser has no resolver and returns the path unchanged. Call it from a PAGE, LAYOUT, or metadata route, which render only on the server. Inside a component that ships to the browser it silently costs you the caching: hydration is a full client re-render, so the bare path overwrites the hashed one and the asset downloads twice. The url stays valid either way, which is why this is a convention rather than a check rule. Under `webjs.basePath`, include the prefix yourself (`asset('/app/public/x.css')`): the framework base-path-prefixes only the urls it emits, so an author-written url is already yours to prefix. Two more constraints: call it INSIDE the render function, because a module-scope call is a side effect the elision analyser reads as client work and it ships the whole module; and mark only files that change with a DEPLOY, because the hash is memoized for the process lifetime, so a `public/` file rewritten in place at runtime would keep its old url while being served `immutable` for a year. Off in dev, so dev output is byte-identical. Only `public/` paths resolve; anything else (and a path that fails to resolve) is returned untouched.
+That emits `/public/app.css?v=<hash>` in production and gets the immutable year; the same url un-marked gets a ~1h cap and can serve stale bytes from a CDN after a deploy until something purges it. `asset()` resolves on the server; the browser has no resolver and returns the path unchanged. Call it from a PAGE, LAYOUT, or metadata route, which render only on the server. Inside a component that ships to the browser it silently costs you the caching: hydration is a full client re-render, so the bare path overwrites the hashed one and the asset downloads twice. The url stays valid either way, so this is a convention rather than a `webjs check` rule (`webjs doctor` does flag the plain form, see below). Under `webjs.basePath`, include the prefix yourself (`asset('/app/public/x.css')`): the framework base-path-prefixes only the urls it emits, so an author-written url is already yours to prefix. Two more constraints: call it INSIDE the render function, because a module-scope call is a side effect the elision analyser reads as client work and it ships the whole module; and mark only files that change with a DEPLOY, because the hash is memoized for the process lifetime, so a `public/` file rewritten in place at runtime would keep its old url while being served `immutable` for a year. Off in dev, so dev output is byte-identical. Only `public/` paths resolve; anything else (and a path that fails to resolve) is returned untouched.
 
 Forgetting it is the one real cost of opt-in, so `webjs doctor` catches it: a page, layout, or error boundary writing a plain `<link rel="stylesheet" href="/public/app.css">` gets a WARN naming the `file:line` and the fix (#1095). It reads your source and rewrites nothing, and it stays quiet about the non-marks that are deliberate: a cross-origin sheet, a `rel="icon"`, a `rel="preload"`, and any `href=${expr}` hole. Same posture as Rails (a `stylesheet_link_tag` helper over a digest manifest) and Remix (a hashed url from the build graph, surfaced through `links()`): take the fingerprint at the point the url is PRODUCED, never by rewriting a rendered document.
 

packages/cli/lib/doctor.js

@@ -969,7 +969,7 @@ async function checkStaticAssetFreshness(appDir) {
   };
 }
 
-// Directories the page/layout walk never descends into (deps, VCS, framework
+// Directories the route-module walk never descends into (deps, VCS, framework
 // and build caches). Mirrors FRESHNESS_IGNORE; kept separate so either walk can
 // change its exclusions without silently moving the other.
 const ROUTE_WALK_IGNORE = new Set(['node_modules', '.git', '.webjs', 'dist', '.next', 'coverage']);
@@ -985,7 +985,16 @@ const ROUTE_WALK_IGNORE = new Set(['node_modules', '.git', '.webjs', 'dist', '.n
  * @type {RegExp}
  */
 const ROUTE_MODULE_RE =
-  /^(?:page|layout|error|not-found|global-error|global-not-found|forbidden|unauthorized|loading)\.(?:js|ts|mjs|mts)$/;
+  /^(?:page|layout|error|not-found|forbidden|unauthorized|loading)\.(?:js|ts|mjs|mts)$/;
+
+/**
+ * The two boundary stems `router.js` registers ONLY at the app root (both are
+ * guarded by `dir === '.'` there). A nested `app/admin/global-error.ts` is never
+ * in the route table and never renders, so scanning one would advise on dead
+ * code, the same defect the `_private` skip exists to avoid.
+ * @type {RegExp}
+ */
+const ROOT_ONLY_MODULE_RE = /^(?:global-error|global-not-found)\.(?:js|ts|mjs|mts)$/;
 
 /**
  * One whole `<link …>` tag. QUOTE-AWARE (`(?:[^>"']|"[^"]*"|'[^']*')*`), the
@@ -1050,12 +1059,14 @@ function parseTagAttrs(tag) {
  *     stripped, since under a sub-path deploy the author writes the prefix
  *     themselves and `resolveAssetUrl` strips it before its own `public/` gate).
  *   - `resolveAssetUrl` would actually fingerprint it. It returns a path
- *     carrying a QUERY or a `..` unchanged, so flagging one would raise a
- *     warning that `asset()` cannot clear: the author wraps it, nothing
- *     changes, the warning stays, and `doctor --strict` can never go green. A
- *     hand-rolled `?v=` cache-buster is exactly what an author who has not
+ *     carrying a QUERY or a `..` unchanged, so wrapping one in `asset()` is a
+ *     runtime NO-OP: the author does the work and the url they ship is
+ *     byte-identical. Advising it would be advising a change that buys nothing.
+ *     A hand-rolled `?v=` cache-buster is exactly what an author who has not
  *     adopted `asset()` is most likely to have written, so this is the common
- *     case, not a corner. Only advise a fix that works.
+ *     case, not a corner. (The warning itself would clear, since this check
+ *     reads the SOURCE shape and a wrapped href is an unquoted hole. Clearing a
+ *     warning without improving the caching is the outcome to avoid.)
  *
  * @param {string} tag
  * @param {string} basePath  the app's normalized `webjs.basePath` (`''` at root)
@@ -1074,9 +1085,8 @@ function unmarkedStylesheetHref(tag, basePath = '') {
   // one `asset()` can actually fingerprint. It strips the base path, cuts at
   // `?` / `#`, DECODES, and only then tests `..` and the `public/` prefix.
   // Testing the raw value instead disagrees at both ends: `/public/%2e%2e/x`
-  // would be flagged although `asset()` refuses it (an unclearable warning,
-  // the very thing this check must never raise), and `/%70ublic/app.css` would
-  // be skipped although `asset()` fingerprints it.
+  // would be flagged although wrapping it changes nothing, and
+  // `/%70ublic/app.css` would be skipped although `asset()` fingerprints it.
   let probe = url;
   if (basePath && probe.startsWith(basePath + '/')) probe = probe.slice(basePath.length);
   const cuts = [probe.indexOf('?'), probe.indexOf('#')].filter((i) => i !== -1);
@@ -1129,29 +1139,73 @@ async function readAppBasePath(appDir) {
 }
 
 /**
- * Commented-out regions, so dead markup is never read as live. Covers the three
- * forms a route module can carry: an HTML comment inside a template, and the JS
- * block and line comments around it. The JS LINE form is the one that actually
- * bites, since commenting out a tag in a `.ts` page is done with `//`, and a
- * warning the author can only clear by deleting a comment is exactly the
- * un-clearable advice this check must never give.
+ * Blank commented-out regions to spaces so dead markup is never read as live,
+ * LENGTH-PRESERVING (newlines kept) so reported line numbers still point at the
+ * real source.
  *
- * The `//` matcher requires the slashes NOT be preceded by `:`, so a `https://`
- * or protocol-relative url inside an href is not mistaken for a comment. Any
- * residual mismatch blanks too much rather than too little, which costs a
- * detection instead of inventing one.
- * @type {RegExp}
+ * A quote-aware WALK rather than a regex, because the two comment families live
+ * in opposite contexts and a flat regex cannot tell them apart. The `<link>`
+ * tags sit inside a template literal, so an HTML comment must be blanked INSIDE
+ * a string, while a JS `//` or block comment is only a comment OUTSIDE one. A
+ * plain `(?<!:)//` matcher gets that second half wrong in the way that matters
+ * most here: it fires on a protocol-relative `href="//cdn/x.css"` and blanks the
+ * rest of the line, so a layout mixing a CDN sheet with a local one, the exact
+ * shape this check targets, goes silently inert.
+ *
+ * Backtick, single and double quotes all count as string context, so a `//`
+ * inside a `${}` hole is not treated as a comment. That fails toward scanning
+ * rather than toward blanking, which costs nothing but a redundant look.
+ *
+ * @param {string} src
+ * @returns {string}
  */
-const COMMENT_RE = /<!--[\s\S]*?-->|\/\*[\s\S]*?\*\/|(?<!:)\/\/[^\n]*/g;
+function blankComments(src) {
+  const blank = (t) => t.replace(/[^\n]/g, ' ');
+  let out = '';
+  let i = 0;
+  /** @type {string} */
+  let quote = '';
+  while (i < src.length) {
+    // An HTML comment is blanked in ANY context: it lives in the template.
+    if (src.startsWith('<!--', i)) {
+      const end = src.indexOf('-->', i + 4);
+      const stop = end === -1 ? src.length : end + 3;
+      out += blank(src.slice(i, stop));
+      i = stop;
+      continue;
+    }
+    const ch = src[i];
+    if (quote) {
+      if (ch === '\\') { out += src.slice(i, i + 2); i += 2; continue; }
+      if (ch === quote) quote = '';
+      out += ch; i++; continue;
+    }
+    if (ch === '"' || ch === "'" || ch === '`') { quote = ch; out += ch; i++; continue; }
+    if (ch === '/' && src[i + 1] === '/') {
+      const nl = src.indexOf('\n', i);
+      const stop = nl === -1 ? src.length : nl;
+      out += blank(src.slice(i, stop));
+      i = stop; continue;
+    }
+    if (ch === '/' && src[i + 1] === '*') {
+      const end = src.indexOf('*/', i + 2);
+      const stop = end === -1 ? src.length : end + 2;
+      out += blank(src.slice(i, stop));
+      i = stop; continue;
+    }
+    out += ch; i++;
+  }
+  return out;
+}
 
 /**
- * Collect every `app/**` page + layout module path, depth-first.
+ * Collect every `app/**` route module that renders markup, depth-first.
  * Best-effort: an unreadable directory contributes nothing rather than throwing.
  * @param {string} dir
  * @param {string[]} [out]
  * @returns {string[]}
  */
-function collectRouteModules(dir, out = []) {
+function collectRouteModules(dir, root = dir, out = []) {
   let entries;
   try { entries = readdirSync(dir, { withFileTypes: true }); } catch { return out; }
   for (const e of entries) {
@@ -1162,14 +1216,15 @@ function collectRouteModules(dir, out = []) {
     // never rendered. Advising on it would be advice about dead code.
     if (e.isDirectory() && e.name.startsWith('_')) continue;
     const abs = join(dir, e.name);
-    if (e.isDirectory()) collectRouteModules(abs, out);
+    if (e.isDirectory()) collectRouteModules(abs, root, out);
     else if (ROUTE_MODULE_RE.test(e.name)) out.push(abs);
+    else if (dir === root && ROOT_ONLY_MODULE_RE.test(e.name)) out.push(abs);
   }
   return out;
 }
 
 /**
- * ADVISORY (#1095): a page or layout hand-writes a `<link rel="stylesheet"
+ * ADVISORY (#1095): a route module hand-writes a `<link rel="stylesheet"
  * href="/public/…">` without `asset()`, so the url is un-versioned and a deploy
  * cannot bust a CDN's copy of it.
  *
@@ -1223,11 +1278,9 @@ async function checkUnmarkedAssetLinks(appDir) {
     // scanned, or the scanner's own case-insensitivity is unreachable exactly
     // where it is needed.
     if (!/<link/i.test(src)) continue;
-    // Blank commented-out regions to spaces, LENGTH-PRESERVING (a newline is
-    // kept as itself) so the reported line numbers still point at the real
-    // source. A commented-out tag emits nothing, so advising on it is advice
-    // about dead markup.
-    const scan = src.replace(COMMENT_RE, (c) => c.replace(/[^\n]/g, ' '));
+    // A commented-out tag emits nothing, so advising on it is advice about
+    // dead markup.
+    const scan = blankComments(src);
     LINK_TAG_RE.lastIndex = 0;
     for (const m of scan.matchAll(LINK_TAG_RE)) {
       const href = unmarkedStylesheetHref(m[0], basePath);
@@ -1238,7 +1291,7 @@ async function checkUnmarkedAssetLinks(appDir) {
     }
   }
   if (findings.length === 0) {
-    return { name, status: 'pass', message: 'every page/layout stylesheet link is content-hashed (or has none)' };
+    return { name, status: 'pass', message: 'every route-module stylesheet link is content-hashed (or has none)' };
   }
   const rel = (f) => relative(appDir, f) || f;
   return {

test/cli/doctor.test.mjs

@@ -1129,3 +1129,32 @@ test('asset-link advisory covers the boundary modules, not just page and layout'
   assert.match(r.message, /ge\.css/);
   assert.match(r.message, /nf\.css/);
 });
+
+test('asset-link advisory scans global-error only at the app root, as the router does', async () => {
+  const nested = tmpDir();
+  // router.js registers global-error / global-not-found only when dir === '.',
+  // so a nested one is never in the route table and never renders.
+  write(nested, 'app/admin/global-error.ts', 'export default () => `<link rel="stylesheet" href="/public/dead.css">`;');
+  assert.equal(byName(await runDoctorChecks(nested, baseOpts()), ASSET_LINK_CHECK).status, 'pass');
+
+  const root = tmpDir();
+  write(root, 'app/global-error.ts', 'export default () => `<link rel="stylesheet" href="/public/ge.css">`;');
+  assert.equal(byName(await runDoctorChecks(root, baseOpts()), ASSET_LINK_CHECK).status, 'warn');
+});
+
+test('asset-link advisory does not treat a // inside an attribute value as a comment', async () => {
+  // A protocol-relative CDN sheet beside a local one is the exact layout this
+  // check targets. A flat `//` comment matcher blanks the rest of that line and
+  // makes the scan silently inert there.
+  for (const [label, markup] of [
+    ['protocol-relative sheet', '<link rel="stylesheet" href="//cdn.example/x.css"><link rel="stylesheet" href="/public/a.css">'],
+    ['preconnect', '<link rel="preconnect" href="//fonts.example"><link rel="stylesheet" href="/public/a.css">'],
+    ['// inside a data attribute', '<link data-x="a//b" rel="stylesheet" href="/public/a.css">'],
+  ]) {
+    const dir = tmpDir();
+    write(dir, 'app/layout.ts', 'export default () => `' + markup + '`;');
+    const r = byName(await runDoctorChecks(dir, baseOpts()), ASSET_LINK_CHECK);
+    assert.equal(r.status, 'warn', label);
+    assert.match(r.message, /a\.css/, label);
+  }
+});

website/app/docs/configuration/page.ts

@@ -47,7 +47,7 @@ webjs routes --json             # structured JSON (matches the MCP list_routes t
     <pre>webjs doctor            # human-readable project-health checklist
 webjs doctor --json     # structured results (each with a stable code) + a summary
 webjs doctor --strict   # also fail the exit on warnings, not just hard failures</pre>
-    <p>Verifies project health: the Node version floor, <code>erasableSyntaxOnly</code>, <code>.env</code> drift, vendor-pin freshness, importmap coherence, <code>@webjsdev/*</code> version coherence, framework resolvability, the git hook, a page/layout elision advisory, and a warning when a page or layout writes a <code>&lt;link rel="stylesheet"&gt;</code> without <code>asset()</code> (so its url is un-versioned and a deploy cannot bust a cached copy). Each result carries a stable machine <code>code</code> (for example <code>NODE_VERSION</code>, <code>TSCONFIG_ERASABLE</code>, <code>IMPORTMAP_COHERENCE</code>) so an agent branches on the failure kind, not the message text. The <code>--json</code> payload is an object <code>&#123; results, summary &#125;</code> (the <code>results</code> array holds the per-check objects, each with its <code>code</code>). By default the exit is non-zero only on a hard <em>toolchain</em> failure; <code>--strict</code> also fails on warnings, so it can gate a fully-clean fix loop the way <code>webjs check --json</code> does.</p>
+    <p>Verifies project health: the Node version floor, <code>erasableSyntaxOnly</code>, <code>.env</code> drift, vendor-pin freshness, importmap coherence, <code>@webjsdev/*</code> version coherence, framework resolvability, the git hook, a page/layout elision advisory, and a warning when a route module writes a <code>&lt;link rel="stylesheet"&gt;</code> without <code>asset()</code> (so its url is un-versioned and a deploy cannot bust a cached copy). Each result carries a stable machine <code>code</code> (for example <code>NODE_VERSION</code>, <code>TSCONFIG_ERASABLE</code>, <code>IMPORTMAP_COHERENCE</code>) so an agent branches on the failure kind, not the message text. The <code>--json</code> payload is an object <code>&#123; results, summary &#125;</code> (the <code>results</code> array holds the per-check objects, each with its <code>code</code>). By default the exit is non-zero only on a hard <em>toolchain</em> failure; <code>--strict</code> also fails on warnings, so it can gate a fully-clean fix loop the way <code>webjs check --json</code> does.</p>
 
     <h3>webjs version</h3>
     <pre>webjs version           # print the installed @webjsdev/cli version