From 5d719a279a3b7cac22f80eb6466973bc4bf83fd1 Mon Sep 17 00:00:00 2001 From: Nils Kanevad Date: Mon, 20 Jul 2026 19:49:36 +0200 Subject: [PATCH] Fix Live accept for Elixir templates in lib/ (#374) * Fix Live accept for Elixir templates in lib/ Wrap and accept search the repo for impeccable variant markers. That search skipped .ex files and the lib/ tree, so Phoenix LiveView markup inside ~H""" blocks never matched and browser Accept returned "Session markers not found". Extend the same EXTENSIONS and searchDirs in live-accept.mjs and live-wrap.mjs. Add a regression test that accepts from lib/my_app_web/components/layouts.ex. * Live: give the source search one owner for template extensions The #374 fix had to patch the same hardcoded EXTENSIONS array in two files because live-wrap.mjs and live-accept.mjs each carried their own copy of the project source walk. The copies had already drifted: same extension list twice, same searchDirs twice, and one realpathSync guarded by try/catch while the other was not. Meanwhile hook-lib.mjs had solved this properly for the design hook in #316/#347 with a configurable `detector.extensions` and suffix matching that handles .blade.php and .html.erb. Live never read it, so a project that taught the hook about .heex still got 'Session markers not found' on Accept. - lib/template-extensions.mjs is the single owner. It holds Live's built-in markup list, the suffix matcher, and the detector.extensions config reader. hook-lib.mjs now imports its normalize/merge/match helpers from here instead of duplicating them, and re-exports matchConfiguredExtension for its existing callers. - Live resolves built-ins PLUS detector.extensions, so teaching the hook about a server template teaches wrap and accept at the same time. - live/source-search.mjs holds the walk both scripts share. Callers pass the one thing that actually differs (skipDirs, fileFilter). Unifying gives live-wrap the guarded realpathSync, so a dangling symlink in the tree no longer throws out of the whole wrap, and makes it skip .impeccable artifacts the way accept already did. - Extensions are matched on filename suffix rather than path.extname, so root.html.heex and show.html.erb resolve. - Drop .exs. Those are Elixir scripts (mix.exs, config/*.exs), never markup, and including them only lets a wrap query match build config. - Fill the Elixir gap in the manual-edit paths, which kept their own allowlists and would have left Live half-working for Phoenix: live-commit-manual-edits.mjs and live-manual-edit-evidence.mjs. Verified the round trip by hand against a Phoenix layout: wrap injects markers into a ~H""" block in lib/**/*.ex, accept carbonizes the chosen variant back out. AI assistance: written with Claude Code. Co-Authored-By: Claude --------- Co-authored-by: Nils Kanevad Co-authored-by: Paul Bakaus Co-authored-by: Claude --- scripts/test-suites.mjs | 11 +- skill/scripts/hook-lib.mjs | 53 ++----- skill/scripts/lib/template-extensions.mjs | 146 +++++++++++++++++++ skill/scripts/live-accept.mjs | 76 +++------- skill/scripts/live-commit-manual-edits.mjs | 3 + skill/scripts/live-manual-edit-evidence.mjs | 7 +- skill/scripts/live-wrap.mjs | 63 ++------ skill/scripts/live/source-search.mjs | 105 ++++++++++++++ tests/live-accept.test.mjs | 37 +++++ tests/live-source-search.test.mjs | 106 ++++++++++++++ tests/template-extensions.test.mjs | 150 ++++++++++++++++++++ 11 files changed, 600 insertions(+), 157 deletions(-) create mode 100644 skill/scripts/lib/template-extensions.mjs create mode 100644 skill/scripts/live/source-search.mjs create mode 100644 tests/live-source-search.test.mjs create mode 100644 tests/template-extensions.test.mjs diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index b47bccb01..8c2ac7adc 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -25,11 +25,11 @@ export const SUITES = { triggers: [ ...COMMON_INFRA_PATTERNS, /^scripts\/(?!benchmark-detector|build-browser-detector|build-extension)/, - /^skill\/(SKILL\.src\.md|agents\/|reference\/|scripts\/(cleanup-deprecated|context|context-signals|critique-storage|design-parser|hook|impeccable-paths|is-generated|lib\/provider|pin))/, + /^skill\/(SKILL\.src\.md|agents\/|reference\/|scripts\/(cleanup-deprecated|context|context-signals|critique-storage|design-parser|hook|impeccable-paths|is-generated|lib\/provider|lib\/template-extensions|pin))/, /^site\/(pages|content|components|layouts)\//, /^README(\.npm)?\.md$/, /^cli\/bin\//, - /^tests\/(build|cleanup-deprecated|cli-ignores|context|context-signals|critique-storage|design-parser|docs-integrity|github-sheriff|hook|hook-build|impeccable-paths|openai-plugin|pin|shiki-theme|skills-cli|target-args|test-suites|windows-path-fix|zip)\.test\.(js|mjs)$/, + /^tests\/(build|cleanup-deprecated|cli-ignores|context|context-signals|critique-storage|design-parser|docs-integrity|github-sheriff|hook|hook-build|impeccable-paths|openai-plugin|pin|shiki-theme|skills-cli|target-args|template-extensions|test-suites|windows-path-fix|zip)\.test\.(js|mjs)$/, /^tests\/lib\//, ], commands: [ @@ -67,6 +67,7 @@ export const SUITES = { 'tests/pin.test.mjs', 'tests/target-args.test.mjs', 'tests/shiki-theme.test.mjs', + 'tests/template-extensions.test.mjs', 'tests/test-suites.test.mjs', 'tests/zip.test.mjs', ], @@ -109,7 +110,7 @@ export const SUITES = { description: 'Fast live-mode unit and local-server integration tests, excluding full browser fixture sweeps.', triggers: [ ...COMMON_INFRA_PATTERNS, - /^skill\/(reference\/live\.md|scripts\/(detect-csp|lib\/is-generated|live\/|live|live-|modern-screenshot|pin|palette))/, + /^skill\/(reference\/live\.md|scripts\/(detect-csp|lib\/is-generated|lib\/template-extensions|live\/|live|live-|modern-screenshot|pin|palette))/, /^tests\/live-/, /^tests\/live-e2e\/(agent|agents\/llm-agent|cli-options|preactions|session|steer|ui)\.mjs$/, /^tests\/live-e2e\/agent-insert\.test\.mjs$/, @@ -148,6 +149,7 @@ export const SUITES = { 'tests/live-server.test.mjs', 'tests/live-session-store.test.mjs', 'tests/live-source-lock.test.mjs', + 'tests/live-source-search.test.mjs', 'tests/live-target-context.test.mjs', 'tests/live-wrap.test.mjs', 'tests/live-wrap-buffer-aware.test.mjs', @@ -163,7 +165,8 @@ export const SUITES = { /^tests\/framework-fixtures\.test\.mjs$/, /^skill\/scripts\/(detect-csp|live-inject|live-wrap)\.mjs$/, /^skill\/scripts\/lib\/is-generated\.mjs$/, - /^skill\/scripts\/live\/sveltekit-adapter\.mjs$/, + /^skill\/scripts\/lib\/template-extensions\.mjs$/, + /^skill\/scripts\/live\/(source-search|sveltekit-adapter)\.mjs$/, ], commands: [ { diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index f90c2f4a8..87343abb8 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -42,6 +42,16 @@ import path from 'node:path'; import { pathToFileURL, fileURLToPath } from 'node:url'; import { extractPlatform, loadContext } from './context.mjs'; import { IMPECCABLE_COMMAND } from './lib/provider.mjs'; +// `detector.extensions` (issue #316) is shared with Live's source search, which +// needs the same answer for `.heex` / `.blade.php` when it hunts for session +// markers. lib/template-extensions.mjs owns the shape; re-exported here because +// hook-lib has been the import site for matchConfiguredExtension since #347. +import { + matchConfiguredExtension, + mergeExtensions, +} from './lib/template-extensions.mjs'; + +export { matchConfiguredExtension }; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -263,49 +273,6 @@ function applyDetectorConfigSource(config, raw) { return config; } -// Extra scanned extensions from `detector.extensions` config. Entries are -// `{ ext, engine }` (engine 'html' | 'text', default 'html' — the common case -// for server-side templates) or bare strings as shorthand. Extensions are -// matched against the end of the filename, not path.extname, so double -// extensions like `.blade.php` and `.html.erb` work (issue #316). -function normalizeExtensionEntries(entries) { - if (!Array.isArray(entries)) return []; - const out = []; - for (const entry of entries) { - const raw = typeof entry === 'string' ? entry : entry?.ext; - if (typeof raw !== 'string') continue; - let ext = raw.trim().toLowerCase(); - if (!ext) continue; - if (!ext.startsWith('.')) ext = `.${ext}`; - const engine = (!(typeof entry === 'string') && entry?.engine === 'text') ? 'text' : 'html'; - out.push({ ext, engine }); - } - return out; -} - -function mergeExtensions(existing, incoming) { - const map = new Map(); - for (const entry of normalizeExtensionEntries(existing)) map.set(entry.ext, entry); - for (const entry of normalizeExtensionEntries(incoming)) map.set(entry.ext, entry); - return Array.from(map.values()); -} - -export function matchConfiguredExtension(filePath, extensions) { - if (!Array.isArray(extensions) || extensions.length === 0) return null; - const name = path.basename(String(filePath || '')).toLowerCase(); - if (!name) return null; - // The longest matching suffix wins, so `.blade.php` beats a broader `.php` - // entry regardless of config order. - let best = null; - for (const entry of normalizeExtensionEntries(extensions)) { - if (name.length > entry.ext.length && name.endsWith(entry.ext) - && (!best || entry.ext.length > best.ext.length)) { - best = entry; - } - } - return best; -} - function applyConfigSource(config, raw) { if (!raw || typeof raw !== 'object') return config; if (Object.prototype.hasOwnProperty.call(raw, 'enabled')) { diff --git a/skill/scripts/lib/template-extensions.mjs b/skill/scripts/lib/template-extensions.mjs new file mode 100644 index 000000000..6a115bd7d --- /dev/null +++ b/skill/scripts/lib/template-extensions.mjs @@ -0,0 +1,146 @@ +/** + * One owner for "which file extensions hold UI markup". + * + * Before this module the answer was spelled out separately in hook-lib.mjs + * (`detector.extensions` config, issue #316) and in live-wrap.mjs / + * live-accept.mjs (a hardcoded `EXTENSIONS` array, duplicated verbatim in both). + * The lists drifted: the hook learned configurable server-template extensions + * while Live kept its six frontend defaults, so a Phoenix project got design + * findings on `.heex` files but `Session markers not found` on Accept (#374). + * + * Extensions are matched against the END OF THE FILENAME, not `path.extname`, + * so double extensions like `.blade.php`, `.html.erb`, and `.html.heex` work. + */ + +import fs from 'node:fs'; +import path from 'node:path'; + +/** + * Built-in markup extensions for Live's wrap/accept source search. + * + * Elixir's `.ex` is here because Phoenix function components put `~H"""` + * templates directly in `lib/**\/*.ex`; `.heex` and `.eex` cover standalone + * templates. `.exs` is deliberately absent: those are Elixir *scripts* + * (`mix.exs`, `config/*.exs`, tests) and never hold markup, so including them + * only gives the wrap query a chance to match build config by accident. + */ +export const LIVE_TEMPLATE_EXTENSIONS = Object.freeze([ + '.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro', + '.ex', '.heex', '.eex', +]); + +/** + * Normalize `detector.extensions` entries to `{ ext, engine }`. + * + * Accepts `{ ext, engine }` objects (engine 'html' | 'text', default 'html' — + * the common case for server-side templates) or bare strings as shorthand. + */ +export function normalizeExtensionEntries(entries) { + if (!Array.isArray(entries)) return []; + const out = []; + for (const entry of entries) { + const raw = typeof entry === 'string' ? entry : entry?.ext; + if (typeof raw !== 'string') continue; + let ext = raw.trim().toLowerCase(); + if (!ext) continue; + if (!ext.startsWith('.')) ext = `.${ext}`; + const engine = (!(typeof entry === 'string') && entry?.engine === 'text') ? 'text' : 'html'; + out.push({ ext, engine }); + } + return out; +} + +export function mergeExtensions(existing, incoming) { + const map = new Map(); + for (const entry of normalizeExtensionEntries(existing)) map.set(entry.ext, entry); + for (const entry of normalizeExtensionEntries(incoming)) map.set(entry.ext, entry); + return Array.from(map.values()); +} + +export function matchConfiguredExtension(filePath, extensions) { + if (!Array.isArray(extensions) || extensions.length === 0) return null; + const name = path.basename(String(filePath || '')).toLowerCase(); + if (!name) return null; + // The longest matching suffix wins, so `.blade.php` beats a broader `.php` + // entry regardless of config order. + let best = null; + for (const entry of normalizeExtensionEntries(extensions)) { + if (name.length > entry.ext.length && name.endsWith(entry.ext) + && (!best || entry.ext.length > best.ext.length)) { + best = entry; + } + } + return best; +} + +/** + * Does this filename end in one of `extensions`? + * + * Suffix matching rather than `path.extname` equality, so a configured + * `.html.erb` matches `show.html.erb` (whose extname is only `.erb`). The + * `name.length > ext.length` guard keeps a file literally named `.heex` from + * counting as a template. + */ +export function matchesTemplateExtension(filePath, extensions) { + const name = path.basename(String(filePath || '')).toLowerCase(); + if (!name) return false; + for (const ext of extensions) { + if (name.length > ext.length && name.endsWith(ext)) return true; + } + return false; +} + +/** + * Built-in Live extensions plus any the project configured for the detector. + * + * Reading `detector.extensions` here is the point: a user who taught the design + * hook about `.blade.php` should not have to teach Live separately. Config + * parsing is intentionally minimal (own the shape, not the whole hook config) + * so this module stays importable from the Live CLI without pulling in + * hook-lib.mjs. + */ +export function resolveLiveTemplateExtensions(cwd = process.cwd()) { + const cached = extensionCache.get(cwd); + if (cached) return cached; + const resolved = readLiveTemplateExtensions(cwd); + extensionCache.set(cwd, resolved); + return resolved; +} + +// live-wrap calls the resolver once per candidate query per pass (up to eight +// times in one CLI run), and every call would otherwise re-read and re-parse +// both config files. Keyed by cwd; a single CLI process never rewrites its own +// config mid-run. +const extensionCache = new Map(); + +/** Test seam: drop the memoized config so a fixture can rewrite config.json. */ +export function clearTemplateExtensionCache() { + extensionCache.clear(); +} + +function readLiveTemplateExtensions(cwd) { + const configured = []; + for (const name of ['config.json', 'config.local.json']) { + const raw = safeReadJson(path.join(cwd, '.impeccable', name)); + const detector = raw?.detector; + if (detector && typeof detector === 'object' && !Array.isArray(detector)) { + configured.push(...normalizeExtensionEntries(detector.extensions)); + } + } + const seen = new Set(LIVE_TEMPLATE_EXTENSIONS); + const out = [...LIVE_TEMPLATE_EXTENSIONS]; + for (const { ext } of configured) { + if (seen.has(ext)) continue; + seen.add(ext); + out.push(ext); + } + return out; +} + +function safeReadJson(filePath) { + try { + return JSON.parse(fs.readFileSync(filePath, 'utf-8')); + } catch { + return null; + } +} diff --git a/skill/scripts/live-accept.mjs b/skill/scripts/live-accept.mjs index 453ddea35..0aa6a6d2e 100644 --- a/skill/scripts/live-accept.mjs +++ b/skill/scripts/live-accept.mjs @@ -16,8 +16,10 @@ import fs from 'node:fs'; import path from 'node:path'; import { isGeneratedFile } from './lib/is-generated.mjs'; -import { IMPECCABLE_DIR, getLiveDir, safeSessionId } from './lib/impeccable-paths.mjs'; +import { getLiveDir, safeSessionId } from './lib/impeccable-paths.mjs'; +import { resolveLiveTemplateExtensions } from './lib/template-extensions.mjs'; import { readBuffer as readManualEditsBuffer, writeBuffer as writeManualEditsBuffer } from './live/manual-edits-buffer.mjs'; +import { NEVER_SOURCE_DIRS, findSourceFile } from './live/source-search.mjs'; import { withSourceLockSync } from './live/source-lock.mjs'; import { applyDeferredSvelteComponentAccepts, @@ -26,7 +28,6 @@ import { removeSvelteComponentSession, } from './live/svelte-component.mjs'; -const EXTENSIONS = ['.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro']; const ACCEPT_LOCK_WAIT_MS = 1_000; // Mirrors VARIANT_ID_PATTERN in live/event-validation.mjs, which gates the same // value arriving over HTTP. @@ -892,66 +893,23 @@ function detectCommentSyntax(filePath) { // --------------------------------------------------------------------------- /** - * `.impeccable` is the critical entry, and it is not cosmetic. - * - * Progressive publication stages each revision as `.impeccable/live/artifacts/ - * -r.`, and those artifacts carry the very marker this search - * looks for. The walk reaches `.` for any project whose source is not under one - * of the privileged dirs above (this repo's own site lives in `site/pages/`), and - * dot-directories sort before letters, so the artifact was found *before* the - * real file. isGeneratedFile then declined the accept, and the agent fell back to - * carbonizing several hundred lines of stylesheet by hand. - * - * Impeccable's own state directory is never project source. Never search it. + * Accept also skips `dist` / `build` outright, where wrap descends into them so + * its `includeGenerated` second pass can report a `generatedMatch`. Accept has + * no such pass: a marker found in build output is only ever a stale copy of the + * marker in source. */ -const SEARCH_SKIP_DIRS = new Set(['node_modules', '.git', 'dist', 'build', IMPECCABLE_DIR]); +const SEARCH_SKIP_DIRS = [...NEVER_SOURCE_DIRS, 'dist', 'build']; function findSessionFile(id, cwd) { - const marker = 'impeccable-variants-start ' + id; - const searchDirs = ['src', 'app', 'pages', 'components', 'public', 'views', 'templates', '.']; - const seen = new Set(); - - for (const dir of searchDirs) { - const absDir = path.join(cwd, dir); - if (!fs.existsSync(absDir)) continue; - const result = searchDir(absDir, marker, seen, 0); - if (result) { - const content = fs.readFileSync(result, 'utf-8'); - return { file: result, content, lines: content.split('\n') }; - } - } - return null; -} - -function searchDir(dir, query, seen, depth) { - if (depth > 5) return null; - let realDir; - try { realDir = fs.realpathSync(dir); } catch { return null; } - if (seen.has(realDir)) return null; - seen.add(realDir); - - let entries; - try { entries = fs.readdirSync(dir, { withFileTypes: true }); } - catch { return null; } - - for (const entry of entries) { - if (!entry.isFile()) continue; - if (!EXTENSIONS.includes(path.extname(entry.name).toLowerCase())) continue; - const filePath = path.join(dir, entry.name); - try { - const content = fs.readFileSync(filePath, 'utf-8'); - if (content.includes(query)) return filePath; - } catch { /* skip */ } - } - - for (const entry of entries) { - if (!entry.isDirectory()) continue; - if (SEARCH_SKIP_DIRS.has(entry.name)) continue; - const result = searchDir(path.join(dir, entry.name), query, seen, depth + 1); - if (result) return result; - } - - return null; + const result = findSourceFile({ + query: 'impeccable-variants-start ' + id, + cwd, + extensions: resolveLiveTemplateExtensions(cwd), + skipDirs: SEARCH_SKIP_DIRS, + }); + if (!result) return null; + const content = fs.readFileSync(result, 'utf-8'); + return { file: result, content, lines: content.split('\n') }; } // --------------------------------------------------------------------------- diff --git a/skill/scripts/live-commit-manual-edits.mjs b/skill/scripts/live-commit-manual-edits.mjs index 8bde46b50..89572e759 100644 --- a/skill/scripts/live-commit-manual-edits.mjs +++ b/skill/scripts/live-commit-manual-edits.mjs @@ -29,6 +29,9 @@ const ROLLBACK_EXTENSIONS = new Set([ '.astro', '.cjs', '.css', + '.eex', + '.ex', + '.heex', '.htm', '.html', '.js', diff --git a/skill/scripts/live-manual-edit-evidence.mjs b/skill/scripts/live-manual-edit-evidence.mjs index dd10e96dc..ee8e2b778 100644 --- a/skill/scripts/live-manual-edit-evidence.mjs +++ b/skill/scripts/live-manual-edit-evidence.mjs @@ -14,7 +14,12 @@ import { isGeneratedFile } from './lib/is-generated.mjs'; import { readBuffer, getBufferPath } from './live/manual-edits-buffer.mjs'; const EVIDENCE_VERSION = 1; -const TEXT_EXTENSIONS = new Set(['.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro', '.js', '.mjs', '.ts']); +const TEXT_EXTENSIONS = new Set([ + '.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro', '.js', '.mjs', '.ts', + // Phoenix keeps `~H"""` markup in .ex alongside standalone .heex/.eex + // templates, so copy edits land in all three. + '.ex', '.heex', '.eex', +]); const SEARCH_DIRS = ['src', 'app', 'pages', 'components', 'public', 'views', 'templates', 'site', 'lib', 'data']; const STRONG_LITERAL_MATCH_LIMIT = 8; const WEAK_LITERAL_MATCH_LIMIT = 4; diff --git a/skill/scripts/live-wrap.mjs b/skill/scripts/live-wrap.mjs index 48d7f7683..430abfeaa 100644 --- a/skill/scripts/live-wrap.mjs +++ b/skill/scripts/live-wrap.mjs @@ -14,15 +14,15 @@ import fs from 'node:fs'; import path from 'node:path'; import { isGeneratedFile } from './lib/is-generated.mjs'; +import { resolveLiveTemplateExtensions } from './lib/template-extensions.mjs'; import { readBuffer as readManualEditsBuffer } from './live/manual-edits-buffer.mjs'; +import { findSourceFile } from './live/source-search.mjs'; import { buildSvelteComponentCssAuthoring, scaffoldSvelteComponentSession, shouldUseSvelteComponentInjection, } from './live/svelte-component.mjs'; -const EXTENSIONS = ['.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro']; - export async function wrapCli() { const args = process.argv.slice(2); @@ -672,56 +672,19 @@ function buildCssAuthoring(styleMode, count) { /** * Search project files for the query string (class name, ID, etc.) * Returns the first matching file path, or null. + * + * Only `node_modules`, `.git`, and `.impeccable` are skipped outright. + * dist/build/out are left to the isGeneratedFile guard so the + * `includeGenerated` second pass can still find the element there and report + * `generatedMatch`. */ function findFileWithQuery(query, cwd, genOpts = {}) { - const searchDirs = ['src', 'app', 'pages', 'components', 'public', 'views', 'templates', '.']; - const seen = new Set(); - - for (const dir of searchDirs) { - const absDir = path.join(cwd, dir); - if (!fs.existsSync(absDir)) continue; - const result = searchDir(absDir, query, seen, 0, genOpts); - if (result) return result; - } - return null; -} - -function searchDir(dir, query, seen, depth, genOpts) { - if (depth > 5) return null; // don't go too deep - const realDir = fs.realpathSync(dir); - if (seen.has(realDir)) return null; - seen.add(realDir); - - let entries; - try { entries = fs.readdirSync(dir, { withFileTypes: true }); } - catch { return null; } - - // Check files first - for (const entry of entries) { - if (!entry.isFile()) continue; - const ext = path.extname(entry.name).toLowerCase(); - if (!EXTENSIONS.includes(ext)) continue; - - const filePath = path.join(dir, entry.name); - if (!genOpts.includeGenerated && isGeneratedFile(filePath, genOpts)) continue; - try { - const content = fs.readFileSync(filePath, 'utf-8'); - if (content.includes(query)) return filePath; - } catch { /* skip unreadable files */ } - } - - // Then recurse into directories. Always skip node_modules and .git (never - // project content). dist/build/out are left to the isGeneratedFile guard so - // the includeGenerated second-pass can still find the element there and - // report `generatedMatch`. - for (const entry of entries) { - if (!entry.isDirectory()) continue; - if (entry.name === 'node_modules' || entry.name === '.git') continue; - const result = searchDir(path.join(dir, entry.name), query, seen, depth + 1, genOpts); - if (result) return result; - } - - return null; + return findSourceFile({ + query, + cwd, + extensions: resolveLiveTemplateExtensions(cwd), + fileFilter: (filePath) => genOpts.includeGenerated || !isGeneratedFile(filePath, genOpts), + }); } /** diff --git a/skill/scripts/live/source-search.mjs b/skill/scripts/live/source-search.mjs new file mode 100644 index 000000000..0b7d26b5d --- /dev/null +++ b/skill/scripts/live/source-search.mjs @@ -0,0 +1,105 @@ +/** + * The project-source walk shared by live-wrap.mjs and live-accept.mjs. + * + * Both scripts need the same thing: find the one project file containing a + * string (wrap looks for the element's class/id/text, accept looks for the + * session's `impeccable-variants-start` marker). They had two near-identical + * copies of the walk, and the copies drifted — same `EXTENSIONS` array declared + * twice, same `searchDirs` array declared twice, one `realpathSync` guarded by + * try/catch and the other not. That drift is what #374 had to patch in two + * places at once. + * + * Callers differ only in how they reject a candidate, so that is the one thing + * this module takes as options (`skipDirs`, `fileFilter`). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { IMPECCABLE_DIR } from '../lib/impeccable-paths.mjs'; +import { matchesTemplateExtension } from '../lib/template-extensions.mjs'; + +/** + * Privileged roots, searched in order, before the catch-all `.` walk. + * + * `lib` is here for Phoenix, whose templates live in `lib/my_app_web/`. It is + * an ordering preference rather than a reachability fix: `.` already recurses + * into `lib`, so the real #374 bug was the extension list, not this array. + */ +export const SOURCE_SEARCH_DIRS = Object.freeze([ + 'src', 'app', 'pages', 'components', 'public', 'views', 'templates', 'lib', '.', +]); + +/** + * Directories that are never project source. + * + * `.impeccable` is the critical entry, and it is not cosmetic. Progressive + * publication stages each revision as `.impeccable/live/artifacts/ + * -r.`, and those artifacts carry the very marker accept + * searches for. The walk reaches `.` for any project whose source is not under + * one of the privileged roots above (this repo's own site lives in + * `site/pages/`), and dot-directories sort before letters, so the artifact was + * found *before* the real file. isGeneratedFile then declined the accept, and + * the agent fell back to carbonizing several hundred lines of stylesheet by + * hand. + */ +export const NEVER_SOURCE_DIRS = Object.freeze(['node_modules', '.git', IMPECCABLE_DIR]); + +const MAX_DEPTH = 5; + +/** + * Walk the project for the first template file whose contents include `query`. + * + * @param {object} opts + * @param {string} opts.query substring to find in file contents + * @param {string} opts.cwd project root + * @param {string[]} opts.extensions filename suffixes that count as templates + * @param {Iterable} [opts.skipDirs] directory names never to descend into + * @param {(filePath: string) => boolean} [opts.fileFilter] return false to reject a candidate + * @returns {string|null} absolute path of the first match + */ +export function findSourceFile({ query, cwd, extensions, skipDirs = NEVER_SOURCE_DIRS, fileFilter }) { + const skip = new Set(skipDirs); + const seen = new Set(); + for (const dir of SOURCE_SEARCH_DIRS) { + const absDir = path.join(cwd, dir); + if (!fs.existsSync(absDir)) continue; + const result = walk(absDir, query, extensions, skip, fileFilter, seen, 0); + if (result) return result; + } + return null; +} + +function walk(dir, query, extensions, skip, fileFilter, seen, depth) { + if (depth > MAX_DEPTH) return null; + // A broken symlink anywhere in the tree used to throw straight out of + // live-wrap's copy of this walk, killing the whole wrap. + let realDir; + try { realDir = fs.realpathSync(dir); } catch { return null; } + if (seen.has(realDir)) return null; + seen.add(realDir); + + let entries; + try { entries = fs.readdirSync(dir, { withFileTypes: true }); } + catch { return null; } + + // Files before directories: a match in the current directory beats one + // nested deeper. + for (const entry of entries) { + if (!entry.isFile()) continue; + if (!matchesTemplateExtension(entry.name, extensions)) continue; + const filePath = path.join(dir, entry.name); + if (fileFilter && !fileFilter(filePath)) continue; + try { + if (fs.readFileSync(filePath, 'utf-8').includes(query)) return filePath; + } catch { /* unreadable, skip */ } + } + + for (const entry of entries) { + if (!entry.isDirectory()) continue; + if (skip.has(entry.name)) continue; + const result = walk(path.join(dir, entry.name), query, extensions, skip, fileFilter, seen, depth + 1); + if (result) return result; + } + + return null; +} diff --git a/tests/live-accept.test.mjs b/tests/live-accept.test.mjs index 50e4bc484..0825e3cf6 100644 --- a/tests/live-accept.test.mjs +++ b/tests/live-accept.test.mjs @@ -703,3 +703,40 @@ describe('live-accept — insert sessions', () => { assert.ok(after.includes('Footer')); }); }); + +describe('live-accept — Elixir templates under lib/', () => { + let tmp; + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'impeccable-accept-elixir-')); + mkdirSync(join(tmp, 'lib', 'my_app_web', 'components'), { recursive: true }); + }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + it('accepts a variant when markers live in a .ex file inside lib/', () => { + const id = 'elixir01'; + const source = `defmodule MyAppWeb.Layouts do + def nav(assigns) do + ~H""" + +
+
+ +
+
+ +
+
+ + """ + end +end +`; + writeFileSync(join(tmp, 'lib', 'my_app_web', 'components', 'layouts.ex'), source); + const result = runAccept(tmp, ['--id', id, '--variant', '1']); + assert.equal(result.handled, true, JSON.stringify(result)); + const after = readFileSync(join(tmp, 'lib', 'my_app_web', 'components', 'layouts.ex'), 'utf-8'); + assert.ok(after.includes('>after')); + assert.ok(!after.includes('impeccable-variants-start')); + assert.ok(!after.includes('data-impeccable-variant="original"')); + }); +}); diff --git a/tests/live-source-search.test.mjs b/tests/live-source-search.test.mjs new file mode 100644 index 000000000..b6be69d2a --- /dev/null +++ b/tests/live-source-search.test.mjs @@ -0,0 +1,106 @@ +import { describe, it, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +import { NEVER_SOURCE_DIRS, SOURCE_SEARCH_DIRS, findSourceFile } from '../skill/scripts/live/source-search.mjs'; +import { LIVE_TEMPLATE_EXTENSIONS } from '../skill/scripts/lib/template-extensions.mjs'; + +const EXTS = LIVE_TEMPLATE_EXTENSIONS; + +describe('live source-search — search roots', () => { + it('puts lib/ ahead of the catch-all . walk', () => { + assert.ok(SOURCE_SEARCH_DIRS.includes('lib')); + assert.ok(SOURCE_SEARCH_DIRS.indexOf('lib') < SOURCE_SEARCH_DIRS.indexOf('.')); + }); + + it('never treats impeccable state as project source', () => { + assert.ok(NEVER_SOURCE_DIRS.includes('.impeccable')); + assert.ok(NEVER_SOURCE_DIRS.includes('node_modules')); + assert.ok(NEVER_SOURCE_DIRS.includes('.git')); + }); +}); + +describe('live source-search — findSourceFile', () => { + let tmp; + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-src-search-')); }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + const write = (rel, body) => { + const abs = join(tmp, rel); + mkdirSync(join(abs, '..'), { recursive: true }); + writeFileSync(abs, body); + return abs; + }; + + it('finds a Phoenix ~H template inside lib/', () => { + const abs = write('lib/my_app_web/components/layouts.ex', 'def nav(a) do\n ~H"""\n \n """\nend\n'); + assert.equal(findSourceFile({ query: 'topbar', cwd: tmp, extensions: EXTS }), abs); + }); + + it('finds a .html.heex template', () => { + const abs = write('lib/my_app_web/controllers/page_html/home.html.heex', '
x
\n'); + assert.equal(findSourceFile({ query: 'hero', cwd: tmp, extensions: EXTS }), abs); + }); + + it('skips .impeccable artifacts that carry the same marker', () => { + // The bug this guards: staged revision artifacts hold the marker too, and + // dot-directories sort before letters in the `.` walk. + write('.impeccable/live/artifacts/abc-r1.html', '\n'); + const real = write('views/home.html', '\n'); + assert.equal(findSourceFile({ query: 'impeccable-variants-start abc', cwd: tmp, extensions: EXTS }), real); + }); + + it('never descends into node_modules', () => { + write('node_modules/pkg/dist/index.html', '
x
\n'); + assert.equal(findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS }), null); + }); + + it('honours an extra skipDirs entry', () => { + write('build/index.html', '
x
\n'); + assert.equal( + findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS, skipDirs: [...NEVER_SOURCE_DIRS, 'build'] }), + null, + ); + }); + + it('honours fileFilter rejections', () => { + write('src/index.html', '
x
\n'); + assert.equal( + findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS, fileFilter: () => false }), + null, + ); + }); + + it('ignores files whose extension is not a template', () => { + write('src/notes.md', 'needle\n'); + assert.equal(findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS }), null); + }); + + it('prefers a privileged root over the catch-all walk', () => { + const preferred = write('src/page.html', '
a
\n'); + write('misc/page.html', '
b
\n'); + assert.equal(findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS }), preferred); + }); + + it('survives a broken symlink instead of throwing', () => { + // live-wrap's copy of this walk called realpathSync unguarded, so one dangling + // link anywhere in the tree took down the whole wrap. + mkdirSync(join(tmp, 'src'), { recursive: true }); + symlinkSync(join(tmp, 'does-not-exist'), join(tmp, 'src', 'dangling')); + const abs = write('src/page.html', '
x
\n'); + assert.equal(findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS }), abs); + }); + + it('does not loop on a self-referential symlink', () => { + mkdirSync(join(tmp, 'src', 'inner'), { recursive: true }); + symlinkSync(join(tmp, 'src'), join(tmp, 'src', 'inner', 'loop')); + assert.equal(findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS }), null); + }); + + it('returns null when nothing matches', () => { + write('src/page.html', '
x
\n'); + assert.equal(findSourceFile({ query: 'needle', cwd: tmp, extensions: EXTS }), null); + }); +}); diff --git a/tests/template-extensions.test.mjs b/tests/template-extensions.test.mjs new file mode 100644 index 000000000..aaf8ce377 --- /dev/null +++ b/tests/template-extensions.test.mjs @@ -0,0 +1,150 @@ +import { describe, it, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +import { + LIVE_TEMPLATE_EXTENSIONS, + clearTemplateExtensionCache, + matchConfiguredExtension, + matchesTemplateExtension, + resolveLiveTemplateExtensions, +} from '../skill/scripts/lib/template-extensions.mjs'; +import { matchConfiguredExtension as fromHookLib } from '../skill/scripts/hook-lib.mjs'; + +describe('template-extensions — built-in list', () => { + it('covers the frontend defaults plus Elixir markup', () => { + for (const ext of ['.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro', '.ex', '.heex', '.eex']) { + assert.ok(LIVE_TEMPLATE_EXTENSIONS.includes(ext), `expected ${ext}`); + } + }); + + it('leaves .exs out — those are Elixir scripts, not templates', () => { + assert.ok(!LIVE_TEMPLATE_EXTENSIONS.includes('.exs')); + assert.ok(!matchesTemplateExtension('mix.exs', LIVE_TEMPLATE_EXTENSIONS)); + assert.ok(!matchesTemplateExtension('config/runtime.exs', LIVE_TEMPLATE_EXTENSIONS)); + }); +}); + +describe('template-extensions — matchesTemplateExtension', () => { + it('matches on filename suffix, not path.extname', () => { + // extname('show.html.erb') is '.erb', so an extname check would miss this. + assert.ok(matchesTemplateExtension('show.html.erb', ['.html.erb'])); + assert.ok(matchesTemplateExtension('lib/app_web/root.html.heex', LIVE_TEMPLATE_EXTENSIONS)); + }); + + it('matches plain extensions and is case-insensitive', () => { + assert.ok(matchesTemplateExtension('Layouts.EX', LIVE_TEMPLATE_EXTENSIONS)); + assert.ok(matchesTemplateExtension('index.HTML', LIVE_TEMPLATE_EXTENSIONS)); + }); + + it('rejects a file whose whole name is the extension', () => { + assert.ok(!matchesTemplateExtension('.heex', LIVE_TEMPLATE_EXTENSIONS)); + }); + + it('rejects non-markup files', () => { + assert.ok(!matchesTemplateExtension('main.css', LIVE_TEMPLATE_EXTENSIONS)); + assert.ok(!matchesTemplateExtension('README.md', LIVE_TEMPLATE_EXTENSIONS)); + }); +}); + +describe('template-extensions — hook-lib parity', () => { + it('hook-lib re-exports the same matchConfiguredExtension', () => { + assert.equal(fromHookLib, matchConfiguredExtension); + }); + + it('still prefers the longest configured suffix', () => { + const match = matchConfiguredExtension('show.blade.php', ['.php', '.blade.php']); + assert.equal(match.ext, '.blade.php'); + assert.equal(match.engine, 'html'); + }); + + it('honours an explicit text engine', () => { + const match = matchConfiguredExtension('mail.txt.erb', [{ ext: '.txt.erb', engine: 'text' }]); + assert.equal(match.engine, 'text'); + }); +}); + +describe('template-extensions — resolveLiveTemplateExtensions', () => { + let tmp; + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'impeccable-tmpl-ext-')); + clearTemplateExtensionCache(); + }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + it('returns the built-ins when there is no config', () => { + assert.deepEqual(resolveLiveTemplateExtensions(tmp), [...LIVE_TEMPLATE_EXTENSIONS]); + }); + + it('folds in detector.extensions so Live inherits what the hook was taught', () => { + mkdirSync(join(tmp, '.impeccable'), { recursive: true }); + writeFileSync( + join(tmp, '.impeccable', 'config.json'), + JSON.stringify({ detector: { extensions: ['.blade.php', { ext: 'twig' }] } }), + ); + const exts = resolveLiveTemplateExtensions(tmp); + assert.ok(exts.includes('.blade.php')); + assert.ok(exts.includes('.twig'), 'a bare string should be normalized to a dotted ext'); + }); + + it('reads config.local.json as well', () => { + mkdirSync(join(tmp, '.impeccable'), { recursive: true }); + writeFileSync( + join(tmp, '.impeccable', 'config.local.json'), + JSON.stringify({ detector: { extensions: ['.slim'] } }), + ); + assert.ok(resolveLiveTemplateExtensions(tmp).includes('.slim')); + }); + + it('never duplicates a built-in', () => { + mkdirSync(join(tmp, '.impeccable'), { recursive: true }); + writeFileSync( + join(tmp, '.impeccable', 'config.json'), + JSON.stringify({ detector: { extensions: ['.heex'] } }), + ); + const exts = resolveLiveTemplateExtensions(tmp); + assert.equal(exts.filter((e) => e === '.heex').length, 1); + }); + + it('survives malformed config without throwing', () => { + mkdirSync(join(tmp, '.impeccable'), { recursive: true }); + writeFileSync(join(tmp, '.impeccable', 'config.json'), '{ not json'); + assert.deepEqual(resolveLiveTemplateExtensions(tmp), [...LIVE_TEMPLATE_EXTENSIONS]); + }); + + it('ignores a detector.extensions that is not an array', () => { + mkdirSync(join(tmp, '.impeccable'), { recursive: true }); + writeFileSync( + join(tmp, '.impeccable', 'config.json'), + JSON.stringify({ detector: { extensions: '.blade.php' } }), + ); + assert.deepEqual(resolveLiveTemplateExtensions(tmp), [...LIVE_TEMPLATE_EXTENSIONS]); + }); +}); + +describe('template-extensions — memoization', () => { + let tmp; + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'impeccable-tmpl-cache-')); + clearTemplateExtensionCache(); + }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + it('serves repeat calls from cache', () => { + const first = resolveLiveTemplateExtensions(tmp); + assert.equal(resolveLiveTemplateExtensions(tmp), first, 'same array instance'); + }); + + it('re-reads config after the cache is cleared', () => { + assert.ok(!resolveLiveTemplateExtensions(tmp).includes('.blade.php')); + mkdirSync(join(tmp, '.impeccable'), { recursive: true }); + writeFileSync( + join(tmp, '.impeccable', 'config.json'), + JSON.stringify({ detector: { extensions: ['.blade.php'] } }), + ); + clearTemplateExtensionCache(); + assert.ok(resolveLiveTemplateExtensions(tmp).includes('.blade.php')); + }); +});