diff --git a/package.json b/package.json index 798bd190e..d80612994 100644 --- a/package.json +++ b/package.json @@ -65,8 +65,6 @@ "test:skill-behavior": "node scripts/run-tests.mjs skill-behavior", "test:live-svelte-adapter-deepseek": "node scripts/run-tests.mjs live-svelte-adapter-deepseek", "smoke:hooks": "node scripts/smoke-provider-hooks.mjs", - "bench:detector": "node scripts/benchmark-detector.mjs", - "bench:detector:browser": "node scripts/benchmark-detector.mjs --browser", "audit": "bun audit --audit-level=moderate", "prepack": "cp README.md README.repo.md && cp README.npm.md README.md", "postpack": "cp README.repo.md README.md && rm README.repo.md", diff --git a/scripts/benchmark-detector.mjs b/scripts/benchmark-detector.mjs deleted file mode 100644 index 1e43cff4a..000000000 --- a/scripts/benchmark-detector.mjs +++ /dev/null @@ -1,583 +0,0 @@ -#!/usr/bin/env node - -import fs from 'node:fs'; -import http from 'node:http'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; - -import { - createBrowserDetector, - createDetectorProfile, - detectHtml, - detectText, - detectUrl, - summarizeDetectorProfile, - walkDir, -} from '../cli/engine/detect-antipatterns.mjs'; - -const __dirname = path.dirname(fileURLToPath(import.meta.url)); -const ROOT = path.resolve(__dirname, '..'); -const FIXTURES = path.join(ROOT, 'tests', 'fixtures', 'antipatterns'); -const BROWSER_FIXTURES = [ - 'cramped-padding.html', - 'quality.html', - 'body-text-viewport-edge.html', -]; - -const MIME = { - '.html': 'text/html; charset=utf-8', - '.js': 'text/javascript; charset=utf-8', - '.css': 'text/css; charset=utf-8', - '.svg': 'image/svg+xml', - '.png': 'image/png', - '.jpg': 'image/jpeg', - '.jpeg': 'image/jpeg', -}; - -function parseArgs(argv) { - const args = { - browser: false, - json: false, - out: null, - quick: false, - }; - for (let i = 0; i < argv.length; i++) { - const arg = argv[i]; - if (arg === '--browser') args.browser = true; - else if (arg === '--json') args.json = true; - else if (arg === '--quick') args.quick = true; - else if (arg === '--out') args.out = argv[++i] || null; - else if (arg.startsWith('--out=')) args.out = arg.slice('--out='.length); - else if (arg === '--help') { - printUsage(); - process.exit(0); - } - } - return args; -} - -function printUsage() { - console.log(`Usage: node scripts/benchmark-detector.mjs [options] - -Options: - --quick Run a small smoke benchmark subset - --browser Include browser-backed URL benchmarks - --json Print the benchmark report as JSON - --out FILE Write the benchmark report JSON to FILE - --help Show this help message`); -} - -function nowMs() { - return typeof performance !== 'undefined' && performance.now - ? performance.now() - : Date.now(); -} - -function roundMs(value) { - return Number(value.toFixed(3)); -} - -function isHtml(filePath) { - const ext = path.extname(filePath).toLowerCase(); - return ext === '.html' || ext === '.htm'; -} - -function rel(filePath) { - return path.relative(ROOT, filePath); -} - -function addEvent(profile, event) { - profile.events.push({ - engine: event.engine || 'unknown', - phase: event.phase || 'unknown', - ruleId: event.ruleId || 'unknown', - target: event.target || '', - ms: Number.isFinite(event.ms) ? event.ms : 0, - findings: Number.isFinite(event.findings) ? event.findings : 0, - }); -} - -async function measureCase({ name, engine, mode, target, run }) { - const profile = createDetectorProfile(); - const started = nowMs(); - try { - const result = await run(profile); - const findings = Array.isArray(result) - ? result.length - : (Number.isFinite(result?.findings) ? result.findings : 0); - return { - name, - engine, - mode, - target, - status: 'ok', - totalMs: roundMs(nowMs() - started), - findings, - profile: summarizeDetectorProfile(profile), - events: profile.events, - }; - } catch (err) { - return { - name, - engine, - mode, - target, - status: 'failed', - totalMs: roundMs(nowMs() - started), - findings: 0, - error: err?.message || String(err), - profile: summarizeDetectorProfile(profile), - events: profile.events, - }; - } -} - -function skippedCase({ name, engine, mode, target, reason }) { - return { - name, - engine, - mode, - target, - status: 'skipped', - totalMs: 0, - findings: 0, - skipReason: reason, - profile: [], - events: [], - }; -} - -async function scanDirectory(files, fastMode, profile) { - const findings = []; - for (const file of files) { - if (!fastMode && isHtml(file)) { - findings.push(...await detectHtml(file, { profile })); - } else { - const content = fs.readFileSync(file, 'utf-8'); - findings.push(...detectText(content, file, { profile })); - } - } - return findings; -} - -function selectQuickFiles(files, predicate, preferredNames) { - const preferred = preferredNames - .map(name => files.find(file => path.basename(file) === name)) - .filter(Boolean); - const fallback = files.filter(predicate).slice(0, preferredNames.length || 2); - return preferred.length ? preferred : fallback; -} - -async function runFileBenchmarks(args) { - const files = walkDir(FIXTURES).sort(); - const htmlFiles = files.filter(isHtml); - const textFiles = files.filter(file => !isHtml(file)); - const selectedText = args.quick - ? textFiles.slice(0, 2) - : textFiles; - const selectedHtml = args.quick - ? selectQuickFiles(htmlFiles, isHtml, ['color.html', 'quality.html']) - : htmlFiles; - const directoryFiles = args.quick - ? [...selectedHtml, ...selectedText].sort() - : files; - - const cases = []; - for (const file of selectedText) { - cases.push(await measureCase({ - name: `detectText:${rel(file)}`, - engine: 'regex', - mode: 'file', - target: rel(file), - run: (profile) => detectText(fs.readFileSync(file, 'utf-8'), file, { profile }), - })); - } - - for (const file of selectedHtml) { - cases.push(await measureCase({ - name: `detectHtml:${rel(file)}`, - engine: 'static-html', - mode: 'file', - target: rel(file), - run: (profile) => detectHtml(file, { profile }), - })); - } - - cases.push(await measureCase({ - name: args.quick ? 'directory-default:quick-fixtures' : 'directory-default:all-fixtures', - engine: 'mixed', - mode: 'directory-default', - target: rel(FIXTURES), - run: (profile) => scanDirectory(directoryFiles, false, profile), - })); - - cases.push(await measureCase({ - name: args.quick ? 'directory-fast:quick-fixtures' : 'directory-fast:all-fixtures', - engine: 'regex', - mode: 'directory-fast', - target: rel(FIXTURES), - run: (profile) => scanDirectory(directoryFiles, true, profile), - })); - - return cases; -} - -function startFixtureServer() { - const server = http.createServer((req, res) => { - let filePath; - const urlPath = req.url?.split('?')[0] || '/'; - if (urlPath.startsWith('/fixtures/')) { - filePath = path.join(ROOT, 'tests', urlPath); - } else if (urlPath === '/js/detect-antipatterns-browser.js') { - filePath = path.join(ROOT, 'cli', 'engine', 'detect-antipatterns-browser.js'); - } else { - res.writeHead(404).end(); - return; - } - try { - const body = fs.readFileSync(filePath); - const ext = path.extname(filePath).toLowerCase(); - res.writeHead(200, { 'Content-Type': MIME[ext] || 'application/octet-stream' }); - res.end(body); - } catch { - res.writeHead(404).end(); - } - }); - - return new Promise((resolve, reject) => { - server.once('error', reject); - server.listen(0, '127.0.0.1', () => { - server.off('error', reject); - const address = server.address(); - resolve({ - server, - baseUrl: `http://127.0.0.1:${address.port}`, - }); - }); - }); -} - -async function closeServer(server) { - await new Promise(resolve => server.close(resolve)); -} - -async function runBrowserBenchmarks(args) { - let serverInfo; - try { - serverInfo = await startFixtureServer(); - } catch (err) { - return [ - skippedCase({ - name: 'browser:fixtures', - engine: 'browser', - mode: 'browser', - target: 'localhost', - reason: `localhost fixture server unavailable: ${err?.message || err}`, - }), - ]; - } - - const cases = []; - const browserFiles = args.quick ? ['quality.html'] : BROWSER_FIXTURES; - - try { - for (const fileName of browserFiles) { - const url = `${serverInfo.baseUrl}/fixtures/antipatterns/${fileName}`; - const fresh = await measureCase({ - name: `detectUrl:fresh-load:${fileName}`, - engine: 'browser', - mode: 'fresh-load', - target: url, - run: (profile) => detectUrl(url, { profile, waitUntil: 'load', settleMs: 100 }), - }); - if (fresh.status === 'failed' && /Could not find Chrome|Failed to launch|executable|spawn|puppeteer/i.test(fresh.error || '')) { - cases.push(skippedCase({ - name: `detectUrl:fresh-load:${fileName}`, - engine: 'browser', - mode: 'fresh-load', - target: url, - reason: `Chromium unavailable: ${fresh.error}`, - })); - } else { - cases.push(fresh); - } - } - - const visualContrastUrl = `${serverInfo.baseUrl}/fixtures/antipatterns/visual-contrast.html`; - cases.push(await measureCase({ - name: 'detectUrl:visual-contrast', - engine: 'browser', - mode: 'visual-contrast', - target: visualContrastUrl, - run: (profile) => detectUrl(visualContrastUrl, { - profile, - waitUntil: 'load', - settleMs: 0, - visualContrast: true, - }), - })); - - cases.push(await measureCase({ - name: 'detectUrl:warm-load', - engine: 'browser', - mode: 'warm-load', - target: serverInfo.baseUrl, - run: async (profile) => { - const detector = await createBrowserDetector({ waitUntil: 'load', settleMs: 100 }); - const findings = []; - try { - for (const fileName of browserFiles) { - const url = `${serverInfo.baseUrl}/fixtures/antipatterns/${fileName}`; - findings.push(...await detector.detectUrl(url, { profile })); - } - } finally { - await detector.close(); - } - return findings; - }, - })); - - cases.push(await measureCase({ - name: 'detectUrl:warm-networkidle0', - engine: 'browser', - mode: 'warm-networkidle0', - target: serverInfo.baseUrl, - run: async (profile) => { - const detector = await createBrowserDetector({ waitUntil: 'load', settleMs: 100 }); - const findings = []; - try { - for (const fileName of browserFiles) { - const url = `${serverInfo.baseUrl}/fixtures/antipatterns/${fileName}`; - findings.push(...await detector.detectUrl(url, { - profile, - waitUntil: 'networkidle0', - settleMs: 0, - })); - } - } finally { - await detector.close(); - } - return findings; - }, - })); - - let puppeteer; - try { - puppeteer = await import('puppeteer'); - } catch (err) { - cases.push(skippedCase({ - name: 'browser:pure-vs-overlay', - engine: 'browser', - mode: 'pure-vs-overlay', - target: serverInfo.baseUrl, - reason: `puppeteer unavailable: ${err?.message || err}`, - })); - return cases; - } - - cases.push(await measureCase({ - name: 'browser:pure-vs-overlay', - engine: 'browser', - mode: 'pure-vs-overlay', - target: serverInfo.baseUrl, - run: async (profile) => { - let browser; - const launchStarted = nowMs(); - try { - browser = await puppeteer.default.launch({ - headless: true, - args: process.env.CI ? ['--no-sandbox', '--disable-setuid-sandbox'] : [], - }); - addEvent(profile, { - engine: 'browser', - phase: 'load', - ruleId: 'launch-browser-overlay-bench', - target: serverInfo.baseUrl, - ms: nowMs() - launchStarted, - }); - } catch (err) { - throw new Error(`Chromium unavailable: ${err?.message || err}`); - } - - let findings = []; - try { - const page = await browser.newPage(); - const url = `${serverInfo.baseUrl}/fixtures/antipatterns/${browserFiles[0]}`; - const browserScript = fs.readFileSync(path.join(ROOT, 'cli', 'engine', 'detect-antipatterns-browser.js'), 'utf-8'); - await page.setViewport({ width: 1280, height: 800 }); - await page.goto(url, { waitUntil: 'load', timeout: 30000 }); - await new Promise(resolve => setTimeout(resolve, 100)); - await page.evaluate(() => { window.__IMPECCABLE_CONFIG__ = { autoScan: false }; }); - await page.evaluate(browserScript); - const pureStarted = nowMs(); - findings = await page.evaluate(() => { - const serialized = window.impeccableDetect({ decorate: false, serialize: true }); - return serialized.flatMap(({ findings }) => findings.map(f => ({ id: f.type, snippet: f.detail }))); - }); - addEvent(profile, { - engine: 'browser', - phase: 'scan', - ruleId: 'pure-detect', - target: url, - ms: nowMs() - pureStarted, - findings: findings.length, - }); - const overlayStarted = nowMs(); - const overlayGroupCount = await page.evaluate(() => window.impeccableScan().length); - addEvent(profile, { - engine: 'browser', - phase: 'scan', - ruleId: 'overlay-scan', - target: url, - ms: nowMs() - overlayStarted, - findings: overlayGroupCount, - }); - await page.close().catch(() => {}); - } finally { - const closeStarted = nowMs(); - await browser.close().catch(() => {}); - addEvent(profile, { - engine: 'browser', - phase: 'load', - ruleId: 'close-browser-overlay-bench', - target: serverInfo.baseUrl, - ms: nowMs() - closeStarted, - }); - } - return findings; - }, - })); - } finally { - await closeServer(serverInfo.server); - } - - return cases.map(testCase => { - if (testCase.engine === 'browser' && testCase.status === 'failed' && /Chromium unavailable|Failed to launch|Could not find Chrome|executable|spawn|puppeteer/i.test(testCase.error || '')) { - return skippedCase({ - name: testCase.name, - engine: testCase.engine, - mode: testCase.mode, - target: testCase.target, - reason: testCase.error, - }); - } - return testCase; - }); -} - -function aggregateEvents(cases) { - const profile = createDetectorProfile(); - for (const testCase of cases) { - if (Array.isArray(testCase.events)) profile.events.push(...testCase.events); - } - return summarizeDetectorProfile(profile); -} - -function makeReport(args, cases) { - const summary = aggregateEvents(cases); - return { - version: 1, - createdAt: new Date().toISOString(), - cwd: ROOT, - quick: args.quick, - browser: args.browser, - cases: cases.map(({ events, ...testCase }) => testCase), - summary, - }; -} - -function pad(value, width) { - const str = String(value); - if (str.length >= width) return str.slice(0, width); - return str + ' '.repeat(width - str.length); -} - -function printRows(rows, columns) { - const header = columns.map(col => pad(col.label, col.width)).join(' '); - console.log(header); - console.log(columns.map(col => '-'.repeat(col.width)).join(' ')); - for (const row of rows) { - console.log(columns.map(col => pad(row[col.key] ?? '', col.width)).join(' ')); - } -} - -function printConsoleReport(report) { - console.log(`Detector benchmark ${report.quick ? '(quick)' : '(full)'}`); - console.log(`Cases: ${report.cases.length}`); - const caseRows = report.cases.map(testCase => ({ - status: testCase.status, - engine: testCase.engine, - mode: testCase.mode, - totalMs: testCase.totalMs, - findings: testCase.findings, - target: testCase.target, - })); - printRows(caseRows, [ - { key: 'status', label: 'Status', width: 8 }, - { key: 'engine', label: 'Engine', width: 12 }, - { key: 'mode', label: 'Mode', width: 20 }, - { key: 'totalMs', label: 'Total ms', width: 10 }, - { key: 'findings', label: 'Findings', width: 8 }, - { key: 'target', label: 'Target', width: 60 }, - ]); - - const skipped = report.cases.filter(testCase => testCase.status === 'skipped'); - for (const testCase of skipped) { - console.log(`Skipped ${testCase.name}: ${testCase.skipReason}`); - } - - console.log('\nSlowest profile groups'); - const slowRows = report.summary.slice(0, 20).map(item => ({ - engine: item.engine, - phase: item.phase, - ruleId: item.ruleId, - calls: item.calls, - totalMs: item.totalMs, - avgMs: item.avgMs, - p95: item.p95, - findings: item.findings, - target: item.target, - })); - printRows(slowRows, [ - { key: 'engine', label: 'Engine', width: 12 }, - { key: 'phase', label: 'Phase', width: 14 }, - { key: 'ruleId', label: 'Rule', width: 28 }, - { key: 'calls', label: 'Calls', width: 8 }, - { key: 'totalMs', label: 'Total ms', width: 10 }, - { key: 'avgMs', label: 'Avg ms', width: 8 }, - { key: 'p95', label: 'P95', width: 8 }, - { key: 'findings', label: 'Finds', width: 7 }, - { key: 'target', label: 'Target', width: 45 }, - ]); -} - -async function main() { - const args = parseArgs(process.argv.slice(2)); - const cases = [ - ...await runFileBenchmarks(args), - ]; - if (args.browser) { - cases.push(...await runBrowserBenchmarks(args)); - } - - const report = makeReport(args, cases); - const json = JSON.stringify(report, null, 2); - if (args.out) { - fs.writeFileSync(path.resolve(args.out), json + '\n'); - } - if (args.json) { - process.stdout.write(json + '\n'); - } else { - printConsoleReport(report); - if (args.out) console.log(`\nWrote JSON report to ${path.resolve(args.out)}`); - } - - if (report.cases.some(testCase => testCase.status === 'failed')) { - process.exitCode = 1; - } -} - -main().catch(err => { - console.error(err?.stack || err?.message || err); - process.exit(1); -}); diff --git a/scripts/build-browser-detector.js b/scripts/build-browser-detector.js index 6f5d3070c..32459dee1 100644 --- a/scripts/build-browser-detector.js +++ b/scripts/build-browser-detector.js @@ -1,49 +1,12 @@ #!/usr/bin/env node /** - * Generates cli/engine/detect-antipatterns-browser.js - * by concatenating the browser-safe detector modules and wrapping them in an IIFE. + * The in-page detector bundle is no longer built here. It is produced by the + * engine repo (the WASM rule core plus a thin DOM adapter) and vendored under + * extension/detector/ by `bun run build:extension`. * - * Run: node scripts/build-browser-detector.js + * This stub keeps `bun run build:browser` a no-op so package scripts and CI + * steps that still call it do not break. */ - -import fs from 'fs'; -import path from 'path'; -import { fileURLToPath } from 'url'; -import { bundleBrowserDetectorModules } from './lib/browser-detector-bundle.js'; - -const __dirname = path.dirname(fileURLToPath(import.meta.url)); -const ROOT = path.resolve(__dirname, '..'); - -const OUTPUT = path.join(ROOT, 'cli/engine/detect-antipatterns-browser.js'); -const SITE_OUTPUT = path.join(ROOT, 'site/public/js/detect-antipatterns-browser.js'); - -const code = bundleBrowserDetectorModules(ROOT); - -const output = `/** - * Anti-Pattern Browser Detector for Impeccable - * Copyright (c) 2026 Paul Bakaus - * SPDX-License-Identifier: Apache-2.0 - * - * GENERATED -- do not edit. Source: cli/engine/browser/injected/index.mjs - * Rebuild: node scripts/build-browser-detector.js - * - * Usage: - * Re-scan: window.impeccableScan() - */ -(function () { -if (typeof window === 'undefined') return; -${code} -})(); -`; - -fs.writeFileSync(OUTPUT, output); -console.log(`Generated ${path.relative(ROOT, OUTPUT)} (${(output.length / 1024).toFixed(1)} KB)`); - -// The site consumes this bundle from its own repo. Only mirror it when that -// checkout is present, so a build here never recreates a stray `site/` tree. -if (fs.existsSync(path.dirname(path.dirname(SITE_OUTPUT)))) { - fs.mkdirSync(path.dirname(SITE_OUTPUT), { recursive: true }); - fs.writeFileSync(SITE_OUTPUT, output); - console.log(`Generated ${path.relative(ROOT, SITE_OUTPUT)} (${(output.length / 1024).toFixed(1)} KB)`); -} +console.log('build:browser: the in-page bundle is produced by the engine repo; vendored under extension/detector by build:extension.'); +process.exit(0); diff --git a/scripts/build.js b/scripts/build.js index d4e238e96..55ea47043 100644 --- a/scripts/build.js +++ b/scripts/build.js @@ -32,7 +32,7 @@ import { verifyPluginAgentRewrite, } from './lib/plugin-paths.js'; import { stageOpenAIPlugin } from './lib/openai-plugin.js'; -import { ANTIPATTERNS } from '../cli/engine/registry/antipatterns.mjs'; +import { ENGINE_TARGETS, binaryName, main as fetchEngineMain, readEngineVersion } from './fetch-engine.mjs'; // Sub-page generation is now handled by Astro content collections. /** @@ -60,8 +60,12 @@ function generateCounts(rootDir, skills, buildDir) { commandCount = activeCommands.length; } - // Count detection rules from the detector registry. - const detectionCount = new Set(ANTIPATTERNS.map(rule => rule.id)).size; + // Count detection rules from the engine's rule registry as vendored by + // build:extension (extension/detector/antipatterns.json). The registry + // lives in the engine repo now, so when that file is absent (fresh + // checkout, no extension build) the detection-count check is skipped + // rather than guessed. + const detectionCount = readDetectionRuleCount(rootDir); // Validate counts in key files const filesToCheck = [ @@ -99,7 +103,7 @@ function generateCounts(rootDir, skills, buildDir) { // qualified "issues" both evaded the old pattern, which is how five // stale counts shipped while the validator reported clean. const detectPattern = /\b(\d+)\s+(deterministic\s+)?(detector\s+)?(checks|patterns|rules|detections|issues)\b/gi; - for (const match of strippedContent.matchAll(detectPattern)) { + for (const match of detectionCount == null ? [] : strippedContent.matchAll(detectPattern)) { const num = parseInt(match[1]); if (match[4] === 'issues' && !match[2]) continue; // plain "issues" is prose, not a count claim if (num !== detectionCount && num > 10) { // ignore small numbers like "3 patterns" @@ -113,10 +117,21 @@ function generateCounts(rootDir, skills, buildDir) { console.error(`\n❌ ${errors} stale count reference(s) found. Update them to match source of truth.`); } - console.log(`✓ Generated counts: ${commandCount} commands, ${detectionCount} detection rules`); + console.log(`✓ Generated counts: ${commandCount} commands, ${detectionCount == null ? 'detection rules unchecked (no extension/detector/antipatterns.json)' : `${detectionCount} detection rules`}`); return errors; } +function readDetectionRuleCount(rootDir) { + const registry = path.join(rootDir, 'extension', 'detector', 'antipatterns.json'); + if (!fs.existsSync(registry)) return null; + try { + const rules = JSON.parse(fs.readFileSync(registry, 'utf-8')); + return new Set((Array.isArray(rules) ? rules : []).map(rule => rule.id)).size || null; + } catch { + return null; + } +} + /** * Guard against plugin/skill version drift (issue #274). The pure comparison * lives in ./lib/validate-plugin-versions.js (so it's unit-tested directly); @@ -496,6 +511,54 @@ function syncRootHookManifests(rootDir) { return synced; } +/** + * Every skill copy in dist gets the engine binaries the launcher looks for + * (`scripts/bin/-/impeccable[.exe]`), so the release zips are + * self-contained for installs without egress. Called only in release mode, + * and only after the root harness dirs and ./plugin have been synced from + * dist: those are git-delivered and must stay launcher-only (the binaries + * are gitignored and the launcher downloads them on first run). + * + * Source: skill/scripts/bin/, filled by scripts/fetch-engine.mjs --all. A + * target that could not be fetched is reported and left out; the launcher + * covers it at run time. + */ +async function stageEngineBinaries(rootDir, distDir) { + await fetchEngineMain(['--all', '--lenient']); + const binRoot = path.join(rootDir, 'skill', 'scripts', 'bin'); + const present = ENGINE_TARGETS.filter(t => fs.existsSync(path.join(binRoot, t, binaryName(t)))); + const missing = ENGINE_TARGETS.filter(t => !present.includes(t)); + if (present.length === 0) { + console.warn(`⚠️ No engine binaries for v${readEngineVersion(rootDir)}; zips ship launcher-only (the launcher downloads on first run).`); + return; + } + let copies = 0; + for (const { provider, configDir } of Object.values(PROVIDERS)) { + const scriptsDir = path.join(distDir, provider, configDir, 'skills', 'impeccable', 'scripts'); + if (!fs.existsSync(scriptsDir)) continue; + for (const target of present) { + const src = path.join(binRoot, target, binaryName(target)); + const dest = path.join(scriptsDir, 'bin', target, binaryName(target)); + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.copyFileSync(src, dest); + fs.chmodSync(dest, 0o755); + copies++; + } + } + console.log(`✓ Staged engine v${readEngineVersion(rootDir)} binaries into dist (${present.join(', ')}; ${copies} copies)${missing.length ? `; missing: ${missing.join(', ')}` : ''}`); +} + +function syncEngineVersionFile(rootDir) { + const version = readEngineVersion(rootDir); + if (!/^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/.test(version)) { + throw new Error(`ENGINE_VERSION must be a semver string, got "${version}"`); + } + const dest = path.join(rootDir, 'skill', 'scripts', 'VERSION'); + const current = fs.existsSync(dest) ? fs.readFileSync(dest, 'utf-8') : null; + if (current !== `${version}\n`) fs.writeFileSync(dest, `${version}\n`); + console.log(`✓ Engine pinned at v${version} (skill/scripts/VERSION)`); +} + const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const ROOT_DIR = path.resolve(__dirname, '..'); @@ -590,6 +653,10 @@ async function build() { const buildDir = path.join(ROOT_DIR, 'build'); + // The launcher reads scripts/VERSION to know which engine release to run + // or download; the root ENGINE_VERSION file is the source of truth for it. + syncEngineVersionFile(ROOT_DIR); + // Read source files (unified skills architecture) const { skills } = readSourceFiles(ROOT_DIR); const patterns = readPatterns(ROOT_DIR); @@ -611,13 +678,6 @@ async function build() { transform(skills, DIST_DIR, { skillsVersion }); } - // Assemble universal directory - assembleUniversal(DIST_DIR); - - // Create ZIP bundles (individual + universal) - await createAllZips(DIST_DIR); - - if (BUILD_OPTIONS.syncRootOutputs) { // Copy all provider outputs to project root for direct GitHub installs and // submodule users. `.codex/` is intentionally excluded: Codex no longer @@ -794,6 +854,22 @@ async function build() { const openAiPluginRoot = stageOpenAIPlugin(ROOT_DIR, DIST_DIR); await createProviderZip(openAiPluginRoot, DIST_DIR, 'openai-plugin'); + // Release builds ship the engine binaries inside every dist skill copy so + // universal.zip is self-contained. This runs after the root harness dirs, + // ./plugin, and the OpenAI plugin were staged from dist: git-delivered + // trees stay launcher-only. + // IMPECCABLE_BUNDLE_ENGINE=0 keeps the release zip launcher-only (the size + // grows by roughly 4 MB per target per provider copy otherwise). + if (BUILD_OPTIONS.syncRootOutputs && process.env.IMPECCABLE_BUNDLE_ENGINE !== '0') { + await stageEngineBinaries(ROOT_DIR, DIST_DIR); + } + + // Assemble universal directory + assembleUniversal(DIST_DIR); + + // Create ZIP bundles (universal) + await createAllZips(DIST_DIR); + // Generate authoritative counts and validate references const countErrors = generateCounts(ROOT_DIR, skills, buildDir); diff --git a/scripts/lib/browser-detector-bundle.js b/scripts/lib/browser-detector-bundle.js deleted file mode 100644 index cd9f7f5f3..000000000 --- a/scripts/lib/browser-detector-bundle.js +++ /dev/null @@ -1,30 +0,0 @@ -import fs from 'node:fs'; -import path from 'node:path'; - -const BROWSER_DETECTOR_MODULES = [ - 'cli/engine/shared/constants.mjs', - 'cli/engine/registry/antipatterns.mjs', - 'cli/engine/shared/color.mjs', - 'cli/engine/shared/fonts.mjs', - 'cli/engine/rules/checks.mjs', - 'cli/engine/browser/injected/index.mjs', -]; - -function browserSafeModule(root, relPath) { - let code = fs.readFileSync(path.join(root, relPath), 'utf-8'); - if (relPath === 'cli/engine/registry/antipatterns.mjs') { - const match = code.match(/const ANTIPATTERNS = \[[\s\S]*?\n\];/); - if (!match) throw new Error('Could not extract browser antipattern registry'); - code = match[0]; - } - code = code.replace(/^import[\s\S]*?;\n/gm, ''); - code = code.replace(/^export\s+\{[^}]*\};\n?/gm, ''); - code = code.replace(/^export\s+\{[\s\S]*?^};\n?/gm, ''); - return `// --- ${relPath} ---\n${code.trim()}\n`; -} - -export function bundleBrowserDetectorModules(root) { - return BROWSER_DETECTOR_MODULES - .map((relPath) => browserSafeModule(root, relPath)) - .join('\n'); -} diff --git a/scripts/lib/transformers/factory.js b/scripts/lib/transformers/factory.js index 195e1b159..2f6a87498 100644 --- a/scripts/lib/transformers/factory.js +++ b/scripts/lib/transformers/factory.js @@ -1,3 +1,4 @@ +import fs from 'fs'; import path from 'path'; import { cleanDir, @@ -359,7 +360,12 @@ export function createTransformer(config) { ensureDir(scriptsOutDir); for (const script of skill.scripts) { const scriptContent = replaceScriptProviderMarker(script.content, placeholderKey, provider); - writeFile(path.join(scriptsOutDir, script.name), scriptContent); + const outPath = path.join(scriptsOutDir, script.name); + writeFile(outPath, scriptContent); + // The launcher must stay executable in every provider copy; a + // plain write would drop the bit and `impeccable context` would + // fail with EACCES on the first session. + if (script.mode) fs.chmodSync(outPath, script.mode); scriptCount++; } } diff --git a/scripts/lib/transformers/hooks.js b/scripts/lib/transformers/hooks.js index 3f0dda32a..596608fb1 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -22,7 +22,7 @@ * correct wherever Claude Code unpacks the plugin. */ -export const IMPECCABLE_HOOK_COMMAND_MARKER = 'skills/impeccable/scripts/hook.mjs'; +export const IMPECCABLE_HOOK_COMMAND_MARKER = 'skills/impeccable/scripts/impeccable'; const TIMEOUT_SECONDS = 5; const STATUS_MESSAGE = 'Checking UI changes'; @@ -34,12 +34,37 @@ const STATUS_MESSAGE = 'Checking UI changes'; const STOP_TIMEOUT_SECONDS = 30; const STOP_STATUS_MESSAGE = 'Design deep pass'; -function stopEntry(command) { +// The hook is a verb of the impeccable launcher that ships in the skill's +// scripts dir: `/impeccable hook` (per-edit and Stop passes) and +// `/impeccable hook-before-edit` (Cursor's preToolUse). The launcher +// runs the platform binary next to it, or downloads it once; no runtime probe +// is needed and there is no Node on the path to check. +export const LAUNCHER_NAME = 'impeccable'; +export const LAUNCHER_NAME_WINDOWS = 'impeccable.cmd'; + +// A hook manifest can be copied into a user-level settings file (issue #399: +// user-level hooks fire in every project, where a project-relative path may +// not exist). Guard the invocation so a missing launcher exits 0 without +// swallowing the hook's real exit code when it is present: the `[ ! -f X ] || +// X verb` form (not `... || true`) preserves the launcher's exit code, so +// Claude's exit-2 blocking signal still reaches the agent. +export const guardedLauncher = (launcherPath, verb = 'hook') => + `[ ! -f "${launcherPath}" ] || "${launcherPath}" ${verb}`; + +// cmd.exe form for harnesses that read a `commandWindows` sibling (Codex +// 0.146.0+ selects it on Windows; issue #452). `exit /b` forwards the +// launcher's errorlevel. Paths keep forward slashes; cmd.exe accepts them in +// quoted paths and it is the form the CLI already writes. +export const windowsLauncherCommand = (launcherCmdPath, verb = 'hook') => + `if exist "${launcherCmdPath}" ("${launcherCmdPath}" ${verb} & exit /b)`; + +function stopEntry(command, commandWindows) { return { hooks: [ { type: 'command', command, + ...(commandWindows ? { commandWindows } : {}), timeout: STOP_TIMEOUT_SECONDS, statusMessage: STOP_STATUS_MESSAGE, }, @@ -47,50 +72,31 @@ function stopEntry(command) { }; } -const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs'; -// The Node major the hook runtime requires, kept equal to the engines floor in -// package.json. The probe and the notice both derive from it so they cannot -// disagree about the supported version. -const NODE_MAJOR_FLOOR = 22; -// A hook manifest can be copied into a user-level settings file (issue #399: -// user-level hooks fire in every project, where a project-relative path may -// not exist). Guard node invocations so a missing file exits 0 without -// swallowing node's real exit code when the file is present. -// -// The runtime is guarded too (issue #410): a `node` on PATH too old for the -// hook's ESM syntax dies while hook.mjs is still being parsed, before the -// script's own always-exit-0 contract can run, so the harness reported a hook -// error on every edit and every Stop. Nothing written in ESM can report that -// condition, so the command string itself checks the version floor first, in -// ES5-only syntax that parses on any node old enough to fail it, and exits 0 -// when the runtime is unsupported or missing. -// -// `notice` reports the dead runtime to the user. It is passed per harness -// because only some have a channel for it, checked against each harness's own -// hook reference on the events we hook: -// Claude Code / Codex: `systemMessage` on stdout is shown to the user -> notice -// Cursor: preToolUse output is permission-shaped and its `user_message` -// renders only on DENY, so warning would block the edit -> probe only -// Grok Build: PostToolUse stdout is ignored; Stop additionalContext -// reaches the model, but the node-version notice has no systemMessage -// channel on this harness -> probe only -// Copilot: output contract unconfirmed; do not guess a shape -> probe only -// -// The clamp avoids `<` and `>` deliberately: Volta's Windows shims run through -// `cmd /C`, which reads an angle bracket in the `-e` payload as redirection, so -// `>=` failed before node ran at all and the guard reported a missing runtime on -// a machine that had a supported one (volta-cli/volta#1791). Newlines break the -// same way, so this payload also has to stay on one line. -const NODE_PROBE = `node -e "process.exit(Math.min(parseInt(process.versions.node,10),${NODE_MAJOR_FLOOR})===${NODE_MAJOR_FLOOR}?0:1)" 2>/dev/null`; -const guardedNode = (hookPath, notice = '') => { - const probe = notice - ? `! { ${NODE_PROBE} || { ${notice}; exit 0; }; }` - : `! ${NODE_PROBE}`; - return `[ ! -f "${hookPath}" ] || ${probe} || node "${hookPath}"`; -}; +const launcherIn = (scriptsDir) => `${scriptsDir}/${LAUNCHER_NAME}`; +const launcherCmdIn = (scriptsDir) => `${scriptsDir}/${LAUNCHER_NAME_WINDOWS}`; -function buildClaudeCompatibleHooks(matcher, hookPath, notice = '') { - const command = guardedNode(hookPath, notice); +const CLAUDE_PROJECT_SCRIPTS = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts'; +const CLAUDE_PLUGIN_SCRIPTS = '${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts'; +const CODEX_PLUGIN_SCRIPTS = '${PLUGIN_ROOT}/skills/impeccable/scripts'; +// Codex reads project hooks from `.codex/hooks.json`, but the skill payload the +// hook invokes lives under the install's own skills dir: a `.codex`-directory +// install keeps it at `.codex/skills/...`, while a `.agents` (Codex repo-skills) +// install keeps it at `.agents/skills/...`. Derive the path from the install dir +// so each generated manifest points at its own payload rather than a hardcoded +// `.agents`; otherwise the guarded hook silently no-ops on `.codex` installs. +const codexProjectScripts = (skillDir) => `${skillDir}/skills/impeccable/scripts`; +const CURSOR_SCRIPTS = '.cursor/skills/impeccable/scripts'; +const GITHUB_PROJECT_SCRIPTS = '$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts'; +// Grok project hooks are relative to the git/workspace root. Claude tool names +// in the matcher (Edit|Write|MultiEdit) alias to Grok's search_replace family. +const GROK_PROJECT_SCRIPTS = '.grok/skills/impeccable/scripts'; + +// `windows: true` adds the `commandWindows` sibling; only Codex-shaped +// consumers honor it, and an unknown key would fail Codex's strict parser if +// it were the other way round, so it stays opt-in per manifest. +function buildClaudeCompatibleHooks(matcher, scriptsDir, { windows = false } = {}) { + const command = guardedLauncher(launcherIn(scriptsDir)); + const commandWindows = windows ? windowsLauncherCommand(launcherCmdIn(scriptsDir)) : undefined; return { PostToolUse: [ { @@ -99,52 +105,21 @@ function buildClaudeCompatibleHooks(matcher, hookPath, notice = '') { { type: 'command', command, + ...(commandWindows ? { commandWindows } : {}), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(command)], + Stop: [stopEntry(command, commandWindows)], }; } -// The message says `on PATH` deliberately: the common cause is a hook shell -// whose PATH misses the version manager, so a user already running Node 22 -// needs to know the hook's PATH is at issue and not their install. Apostrophes -// cannot appear in it, since it travels inside a single-quoted shell string. -const NODE_NOTICE_TEXT = `The impeccable design hook is not running: no Node ${NODE_MAJOR_FLOOR} or newer on PATH. ` - + 'Install one, or remove the impeccable hook from your harness settings.'; -// Claude Code and Codex both read `systemMessage`, so one payload serves both. -// The marker under ~/.impeccable holds it to one notice per machine (not per -// harness or per edit), and printf runs only after the marker write succeeds, -// so an unwritable HOME degrades to silence rather than a notice on every edit. -const SYSTEM_MESSAGE_NOTICE = 'D="$HOME/.impeccable"; [ -f "$D/node-unsupported" ] || ' - + '{ mkdir -p "$D" 2>/dev/null && : > "$D/node-unsupported" 2>/dev/null && ' - + `printf '%s' '{"systemMessage":"${NODE_NOTICE_TEXT}"}'; }`; -const CLAUDE_PLUGIN_HOOK = '${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs'; -const CODEX_PLUGIN_HOOK = '${PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs'; -// Codex reads project hooks from `.codex/hooks.json`, but the skill payload the -// hook invokes lives under the install's own skills dir: a `.codex`-directory -// install keeps it at `.codex/skills/...`, while a `.agents` (Codex repo-skills) -// install keeps it at `.agents/skills/...`. Derive the path from the install dir -// so each generated manifest points at its own payload rather than a hardcoded -// `.agents` — otherwise the guarded hook silently no-ops on `.codex` installs. -const codexProjectHook = (skillDir) => `${skillDir}/skills/impeccable/scripts/hook.mjs`; -const CURSOR_BEFORE_EDIT_SCRIPT = '.cursor/skills/impeccable/scripts/hook-before-edit.mjs'; -const GITHUB_PROJECT_HOOK = '$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts/hook.mjs'; -// Grok project hooks are relative to the git/workspace root. Claude tool names -// in the matcher (Edit|Write|MultiEdit) alias to Grok's search_replace family. -const GROK_PROJECT_HOOK = '.grok/skills/impeccable/scripts/hook.mjs'; - export function buildClaudeSettingsManifest() { return { description: 'Impeccable design detector: immediate-tier checks after Edit/Write on UI files, full-rule deep pass on Stop.', - hooks: buildClaudeCompatibleHooks( - 'Edit|Write', - CLAUDE_PROJECT_HOOK, - SYSTEM_MESSAGE_NOTICE, - ), + hooks: buildClaudeCompatibleHooks('Edit|Write', CLAUDE_PROJECT_SCRIPTS), }; } @@ -156,11 +131,7 @@ export function buildClaudeSettingsManifest() { // than `hooks`, failing the whole manifest (issue #330). export function buildClaudePluginHooksManifest() { return { - hooks: buildClaudeCompatibleHooks( - 'Edit|Write', - CLAUDE_PLUGIN_HOOK, - SYSTEM_MESSAGE_NOTICE, - ), + hooks: buildClaudeCompatibleHooks('Edit|Write', CLAUDE_PLUGIN_SCRIPTS), }; } @@ -169,11 +140,7 @@ export function buildClaudePluginHooksManifest() { // instead of relying on its Claude compatibility alias. export function buildCodexPluginHooksManifest() { return { - hooks: buildClaudeCompatibleHooks( - 'Edit|Write|apply_patch', - CODEX_PLUGIN_HOOK, - SYSTEM_MESSAGE_NOTICE, - ), + hooks: buildClaudeCompatibleHooks('Edit|Write|apply_patch', CODEX_PLUGIN_SCRIPTS, { windows: true }), }; } @@ -181,13 +148,8 @@ export function buildCodexPluginHooksManifest() { // emitted command points at that install's payload. Defaults to `.codex` for the // Codex provider, whose self-consistent bundle keeps the skill at `.codex/skills`. export function buildCodexHooksManifest(skillDir = '.codex') { - const hookPath = codexProjectHook(skillDir); return { - hooks: buildClaudeCompatibleHooks( - 'Edit|Write|apply_patch', - hookPath, - SYSTEM_MESSAGE_NOTICE, - ), + hooks: buildClaudeCompatibleHooks('Edit|Write|apply_patch', codexProjectScripts(skillDir), { windows: true }), }; } @@ -197,7 +159,7 @@ export function buildCursorHooksManifest() { hooks: { preToolUse: [ { - command: guardedNode(CURSOR_BEFORE_EDIT_SCRIPT), + command: guardedLauncher(launcherIn(CURSOR_SCRIPTS), 'hook-before-edit'), timeout: TIMEOUT_SECONDS, }, ], @@ -224,7 +186,7 @@ export function buildGitHubHooksManifest() { { type: 'command', matcher: 'edit|create|apply_patch', - bash: guardedNode(GITHUB_PROJECT_HOOK), + bash: guardedLauncher(launcherIn(GITHUB_PROJECT_SCRIPTS)), timeoutSec: TIMEOUT_SECONDS, }, ], @@ -239,7 +201,7 @@ export function buildGitHubHooksManifest() { // https://docs.x.ai/build/features/hooks export function buildGrokHooksManifest() { return { - hooks: buildClaudeCompatibleHooks('Edit|Write|MultiEdit', GROK_PROJECT_HOOK), + hooks: buildClaudeCompatibleHooks('Edit|Write|MultiEdit', GROK_PROJECT_SCRIPTS), }; } diff --git a/scripts/lib/utils.js b/scripts/lib/utils.js index d96eff378..69b2b38bc 100644 --- a/scripts/lib/utils.js +++ b/scripts/lib/utils.js @@ -9,18 +9,11 @@ import path from 'path'; // New installs write project config at .impeccable/live/config.json instead. export const PER_PROJECT_SCRIPT_ARTIFACTS = new Set(['config.json']); -const DETECTOR_BUNDLE_DIR = 'cli/engine'; - -// Detector source files that live OUTSIDE `cli/engine` but are imported by the -// bundled engine. `cli/engine/cli/main.mjs` imports `../../lib/impeccable-config.mjs`, -// which in the source CLI resolves to `cli/lib/impeccable-config.mjs`. The detector -// bundle copies `cli/engine/**` to `scripts/detector/**`, so from the bundled -// `scripts/detector/cli/main.mjs` that same `../../lib/...` import resolves to -// `scripts/lib/impeccable-config.mjs`. Copy the dependency there or the bundled -// detector fails at import time with "Cannot find module .../lib/impeccable-config.mjs". -const DETECTOR_EXTERNAL_DEPS = [ - { src: 'cli/lib/impeccable-config.mjs', dest: 'lib/impeccable-config.mjs' }, -]; +// Platform binaries under `scripts/bin/-/` are fetched per machine +// (scripts/fetch-engine.mjs) and never part of the source skill read: the +// release build stages them into provider output separately, and the launcher +// downloads them on first run when they are absent. +export const SKILL_BINARY_DIR = 'bin'; // Walk the harness-dir skill tree and return any per-project script // artifacts found, ready for restoration after a full sync rm+recopy. @@ -52,46 +45,6 @@ export function restorePerProjectArtifacts(rootDir, stashed) { } } -function readDetectorBundleScripts(rootDir) { - const detectorDir = path.join(rootDir, DETECTOR_BUNDLE_DIR); - if (!fs.existsSync(detectorDir)) return []; - - const scripts = []; - const walk = (dir) => { - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const entryPath = path.join(dir, entry.name); - if (entry.isDirectory()) { - walk(entryPath); - continue; - } - if (!entry.isFile()) continue; - const relPath = path.relative(detectorDir, entryPath).split(path.sep).join('/'); - scripts.push({ - name: `detector/${relPath}`, - content: fs.readFileSync(entryPath, 'utf-8'), - filePath: entryPath, - generated: true, - }); - } - }; - walk(detectorDir); - - // Pull in engine dependencies that live outside the bundle dir so the - // generated detector is self-contained (see DETECTOR_EXTERNAL_DEPS). - for (const { src, dest } of DETECTOR_EXTERNAL_DEPS) { - const srcPath = path.join(rootDir, src); - if (!fs.existsSync(srcPath)) continue; - scripts.push({ - name: dest, - content: fs.readFileSync(srcPath, 'utf-8'), - filePath: srcPath, - generated: true, - }); - } - - return scripts; -} - function readSkillScripts(scriptsDir) { const scripts = []; @@ -102,6 +55,7 @@ function readSkillScripts(scriptsDir) { for (const entry of entries) { const entryPath = path.join(dir, entry.name); if (entry.isDirectory()) { + if (dir === scriptsDir && entry.name === SKILL_BINARY_DIR) continue; walk(entryPath); continue; } @@ -109,10 +63,13 @@ function readSkillScripts(scriptsDir) { if (PER_PROJECT_SCRIPT_ARTIFACTS.has(entry.name)) continue; const relPath = path.relative(scriptsDir, entryPath).split(path.sep).join('/'); + // `mode` travels with the entry so the launcher keeps its executable + // bit in every provider copy (see writeScriptFile in the transformer). scripts.push({ name: relPath, content: fs.readFileSync(entryPath, 'utf-8'), filePath: entryPath, + mode: fs.statSync(entryPath).mode & 0o777, }); } }; @@ -242,8 +199,8 @@ export function readFilesRecursive(dir, fileList = []) { * The source manifest is `SKILL.src.md`, NOT `SKILL.md`, on purpose: the * `vercel-labs/skills` CLI discovers a skill by finding a literal `SKILL.md` * and copies that directory verbatim. If `skill/SKILL.md` existed, `npx skills` - * would install the UNCOMPILED source (unresolved `{{placeholders}}`, no vendored - * detector). Naming it `SKILL.src.md` hides it from discovery so the CLI falls + * would install the UNCOMPILED source (unresolved `{{placeholders}}` and + * `{{scripts_path}}`). Naming it `SKILL.src.md` hides it from discovery so the CLI falls * through to a compiled harness dir (`.agents/skills/impeccable`) instead. */ export function readSourceFiles(rootDir) { @@ -280,7 +237,6 @@ export function readSourceFiles(rootDir) { if (fs.existsSync(scriptsDir)) { scripts.push(...readSkillScripts(scriptsDir)); } - scripts.push(...readDetectorBundleScripts(rootDir)); const agents = []; const agentsDir = path.join(skillDir, 'agents'); @@ -676,13 +632,15 @@ export function replacePlaceholders(content, provider, commandNames = [], allSki // Replace `/skillname` invocations with the correct command prefix for this provider // (e.g., `/normalize` → `$normalize` for Codex). Require the slash to be - // outside a path or URL so `.github/hooks/impeccable.json` and - // `.codex/skills/impeccable` remain untouched. + // outside a path or URL so `.github/hooks/impeccable.json`, + // `.codex/skills/impeccable`, and the launcher invocation + // `{{scripts_path}}/impeccable ` (a `}}`-preceded path segment, + // resolved after this pass) remain untouched. if (cmdPrefix !== '/' && allSkillNames.length > 0) { const sorted = [...allSkillNames].sort((a, b) => b.length - a.length); for (const name of sorted) { result = result.replace( - new RegExp(`(?-])\\/(?=${escapeRegex(name)}(?:[^a-zA-Z0-9_-]|$))`, 'g'), cmdPrefix ); } @@ -695,9 +653,10 @@ export function replacePlaceholders(content, provider, commandNames = [], allSki * Render the one explicit provider marker allowed in executable skill scripts. * * Do not run replacePlaceholders() across JavaScript source: slash-command - * heuristics can collide with regex literals and runtime paths. Scripts import - * their command prefix from lib/provider.mjs, whose declaration is replaced - * here by an exact string match. + * heuristics can collide with regex literals and runtime paths. Only exact + * marker lines are replaced. The shipped scripts today (the launcher and the + * page JS) carry no marker; the binary derives its provider from its install + * path or IMPECCABLE_PROVIDER_ID at run time. */ export function replaceScriptProviderMarker(content, provider, buildProvider = provider) { const placeholders = PROVIDER_PLACEHOLDERS[provider] || PROVIDER_PLACEHOLDERS.cursor; diff --git a/scripts/smoke-provider-hooks.mjs b/scripts/smoke-provider-hooks.mjs index e86b34482..2f7b82c11 100644 --- a/scripts/smoke-provider-hooks.mjs +++ b/scripts/smoke-provider-hooks.mjs @@ -48,24 +48,24 @@ const providerSmoke = { fixture: 'src/__impeccable_provider_smoke_claude.html', confirmedFixture: 'src/__impeccable_provider_smoke_confirmed_claude.html', agentChoiceFixture: 'src/__impeccable_provider_smoke_font_choice_claude.html', - admin: '.claude/skills/impeccable/scripts/hook-admin.mjs', - hook: '.claude/skills/impeccable/scripts/hook.mjs', + launcher: '.claude/skills/impeccable/scripts/impeccable', + hookVerb: 'hook', event: (file) => postToolUseEvent('confirmed-claude', file, 'Edit'), }, codex: { fixture: 'src/__impeccable_provider_smoke_codex.html', confirmedFixture: 'src/__impeccable_provider_smoke_confirmed_codex.html', agentChoiceFixture: 'src/__impeccable_provider_smoke_font_choice_codex.html', - admin: '.agents/skills/impeccable/scripts/hook-admin.mjs', - hook: '.agents/skills/impeccable/scripts/hook.mjs', + launcher: '.agents/skills/impeccable/scripts/impeccable', + hookVerb: 'hook', event: (file) => postToolUseEvent('confirmed-codex', file, 'apply_patch'), }, cursor: { fixture: 'src/__impeccable_provider_smoke_cursor.html', confirmedFixture: 'src/__impeccable_provider_smoke_confirmed_cursor.html', agentChoiceFixture: 'src/__impeccable_provider_smoke_font_choice_cursor.html', - admin: '.cursor/skills/impeccable/scripts/hook-admin.mjs', - hook: '.cursor/skills/impeccable/scripts/hook-before-edit.mjs', + launcher: '.cursor/skills/impeccable/scripts/impeccable', + hookVerb: 'hook-before-edit', event: (file) => ({ hook_event_name: 'preToolUse', cwd: targetRepo, @@ -393,7 +393,7 @@ function stripImpeccableHookEntry(entry) { } function containsImpeccableHook(value) { - if (typeof value === 'string') return value.includes('skills/impeccable/scripts/hook') || value.includes('.cursor/pre-log.mjs'); + if (typeof value === 'string') return value.includes('skills/impeccable/scripts/impeccable') || value.includes('.cursor/pre-log.mjs'); if (Array.isArray(value)) return value.some(containsImpeccableHook); if (value && typeof value === 'object') return Object.values(value).some(containsImpeccableHook); return false; @@ -403,24 +403,19 @@ function verifyInstallShape() { const claude = readText('.claude/settings.local.json'); const codex = readText('.codex/hooks.json'); const cursor = readText('.cursor/hooks.json'); - assertCount(claude, '.claude/skills/impeccable/scripts/hook.mjs', 1, 'Claude hook.mjs'); - assertCount(codex, '.agents/skills/impeccable/scripts/hook.mjs', 1, 'Codex hook.mjs'); - assertCount(cursor, '.cursor/skills/impeccable/scripts/hook-before-edit.mjs', 1, 'Cursor preToolUse'); - assertCount(cursor, '.cursor/skills/impeccable/scripts/hook-after-edit.mjs', 0, 'Cursor afterFileEdit'); - assertCount(cursor, '.cursor/skills/impeccable/scripts/hook-stop.mjs', 0, 'Cursor stop'); + assertCount(claude, '.claude/skills/impeccable/scripts/impeccable" hook', 1, 'Claude hook'); + assertCount(codex, '.agents/skills/impeccable/scripts/impeccable" hook', 1, 'Codex hook'); + assertCount(cursor, '.cursor/skills/impeccable/scripts/impeccable" hook-before-edit', 1, 'Cursor preToolUse'); for (const text of [claude, codex, cursor]) { - if (text.includes('hook-probe.mjs')) throw new Error('hook-probe.mjs still appears in hook manifests'); + if (text.includes('.mjs')) throw new Error('a Node hook script still appears in hook manifests'); } for (const rel of [ - '.claude/skills/impeccable/scripts/hook.mjs', - '.claude/skills/impeccable/scripts/hook-lib.mjs', - '.claude/skills/impeccable/scripts/detector/cli/main.mjs', - '.agents/skills/impeccable/scripts/hook.mjs', - '.agents/skills/impeccable/scripts/hook-lib.mjs', - '.agents/skills/impeccable/scripts/detector/cli/main.mjs', - '.cursor/skills/impeccable/scripts/hook-before-edit.mjs', - '.cursor/skills/impeccable/scripts/hook-lib.mjs', - '.cursor/skills/impeccable/scripts/detector/cli/main.mjs', + '.claude/skills/impeccable/scripts/impeccable', + '.claude/skills/impeccable/scripts/VERSION', + '.agents/skills/impeccable/scripts/impeccable', + '.agents/skills/impeccable/scripts/VERSION', + '.cursor/skills/impeccable/scripts/impeccable', + '.cursor/skills/impeccable/scripts/VERSION', ]) { assertPath(join(targetRepo, rel), rel); } @@ -484,7 +479,7 @@ function runDirectContractChecks() { clearRuntimeState(); const file = writeBadFixture(directSmokeFile); const env = { IMPECCABLE_HOOK_LOG: join(smokeDir, 'direct.ndjson') }; - const claude = run('node', ['.claude/skills/impeccable/scripts/hook.mjs'], { + const claude = run('.claude/skills/impeccable/scripts/impeccable', ['hook'], { cwd: targetRepo, env, logName: 'direct-claude.log', @@ -493,7 +488,7 @@ function runDirectContractChecks() { requireFinding('direct Claude hook', `${claude.stdout}\n${readMaybe(join(smokeDir, 'direct.ndjson'))}`); clearRuntimeState(); - const codex = run('node', ['.agents/skills/impeccable/scripts/hook.mjs'], { + const codex = run('.agents/skills/impeccable/scripts/impeccable', ['hook'], { cwd: targetRepo, env, logName: 'direct-codex.log', @@ -502,7 +497,7 @@ function runDirectContractChecks() { requireFinding('direct Codex hook', `${codex.stdout}\n${readMaybe(join(smokeDir, 'direct.ndjson'))}`); clearRuntimeState(); - const pre = run('node', ['.cursor/skills/impeccable/scripts/hook-before-edit.mjs'], { + const pre = run('.cursor/skills/impeccable/scripts/impeccable', ['hook-before-edit'], { cwd: targetRepo, env, logName: 'direct-cursor-before.log', @@ -543,8 +538,8 @@ function runConfirmedExceptionForProvider(provider) { requireRuleFinding(`${provider} confirmed exception first hook`, `${first.stdout}\n${first.stderr}\n${readMaybe(join(smokeDir, beforeLog))}`, 'overused-font'); assertNoSpecificFontIgnoreConfig(provider); - run('node', [ - providerSmoke[provider].admin, + run(providerSmoke[provider].launcher, [ + 'hooks', 'ignore-value', 'overused-font', 'Roboto', @@ -745,7 +740,7 @@ function readSharedHookConfig() { function runInstalledProviderHook(provider, file, logName) { const smoke = providerSmoke[provider]; const env = { IMPECCABLE_HOOK_LOG: join(smokeDir, logName) }; - return run('node', [smoke.hook], { + return run(smoke.launcher, [smoke.hookVerb], { cwd: targetRepo, env, logName: `direct-${provider}-confirmed-${logName.replace(/\.ndjson$/, '.log')}`, diff --git a/tests/build.test.js b/tests/build.test.js index 6f6865237..073fc21a6 100644 --- a/tests/build.test.js +++ b/tests/build.test.js @@ -374,65 +374,38 @@ Please audit {{target}} for technical quality. Ask {{model}} for help.`; }); }); -// Resolve a relative import specifier against the importer's bundle-relative -// path, mirroring Node ESM resolution against the set of bundled script names. -// Returns the matching bundled name, or null if nothing resolves. -function resolveBundledImport(importerName, specifier, names) { - const dirParts = importerName.split('/').slice(0, -1); - const parts = dirParts.concat(specifier.split('/')); - const resolved = []; - for (const part of parts) { - if (part === '' || part === '.') continue; - if (part === '..') { resolved.pop(); continue; } - resolved.push(part); - } - const base = resolved.join('/'); - // ESM needs an explicit extension, but be tolerant of extensionless and - // index specifiers so the check tracks real module-resolution behavior. - const candidates = [base, `${base}.mjs`, `${base}.js`, `${base}/index.mjs`, `${base}/index.js`]; - return candidates.find((c) => names.has(c)) || null; -} - -// Regression guard for issue #254: the bundled detector imported -// `../../lib/impeccable-config.mjs`, a file that lives outside `cli/engine` and -// was never copied into the bundle, so `/impeccable critique` crashed with -// "Cannot find module .../lib/impeccable-config.mjs". This walks every bundled -// script and asserts each relative import resolves to another bundled file, so -// any future out-of-bundle dependency fails the build instead of the user. -describe('bundled skill scripts are self-contained', () => { +// The skill's scripts dir ships the launcher, its Windows twin, the pinned +// engine VERSION, the page JS, and command-metadata.json. Nothing else: the +// verbs live in the engine binary the launcher runs, and platform binaries +// (scripts/bin/) are fetched per machine, never read as source. +describe('skill scripts payload', () => { const ROOT_DIR = process.cwd(); const { skills } = utils.readSourceFiles(ROOT_DIR); const scripts = skills[0]?.scripts ?? []; - const jsScripts = scripts.filter((s) => /\.(mjs|js)$/.test(s.name)); const names = new Set(scripts.map((s) => s.name)); - // Static `import ... from '...'` and re-export `export ... from '...'` only; - // dynamic `import()` of computed paths (e.g. detect.mjs) is out of scope. - const importRe = /(?:^|[\s;])(?:import|export)\b[^'"`]*?\bfrom\s*['"]([^'"]+)['"]/g; - - // Drop comments first so an example like `// import ... from '...'` in a - // doc comment (detector/node/file-system.mjs has one) isn't read as a real import. - const stripComments = (src) => src.replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/.*$/gm, ''); - - test('the detector bundle includes its config dependency', () => { - expect(names.has('lib/impeccable-config.mjs')).toBe(true); + test('ships the launcher, VERSION, page JS, and command metadata', () => { + for (const expected of [ + 'impeccable', 'impeccable.cmd', 'VERSION', 'command-metadata.json', + 'live-browser.js', 'live-browser-dom.js', 'live-browser-session.js', 'modern-screenshot.umd.js', + ]) { + expect(names.has(expected)).toBe(true); + } }); - test('every relative import resolves to a bundled file', () => { - const broken = []; - for (const script of jsScripts) { - const source = stripComments(script.content); - importRe.lastIndex = 0; - let match; - while ((match = importRe.exec(source)) !== null) { - const specifier = match[1]; - if (!specifier.startsWith('.')) continue; // bare/node specifiers - if (!resolveBundledImport(script.name, specifier, names)) { - broken.push(`${script.name} -> ${specifier}`); - } - } - } - expect(broken).toEqual([]); + test('ships no Node entry points and no bundled detector', () => { + expect([...names].filter((n) => n.endsWith('.mjs') || n.startsWith('detector/') || n.startsWith('lib/'))).toEqual([]); + }); + + test('never reads platform binaries as source', () => { + expect([...names].filter((n) => n.startsWith('bin/'))).toEqual([]); + }); + + test('the launcher is executable and VERSION matches ENGINE_VERSION', () => { + const launcher = scripts.find((s) => s.name === 'impeccable'); + expect(launcher.mode & 0o111).not.toBe(0); + const version = scripts.find((s) => s.name === 'VERSION'); + expect(version.content.trim()).toBe(fs.readFileSync(path.join(ROOT_DIR, 'ENGINE_VERSION'), 'utf-8').trim()); }); }); @@ -688,7 +661,7 @@ describe('agent bodies resolve placeholders on every surface that ships them', ( test('the asset producer ships a runnable embed-prompt command, never the raw token', () => { for (const [relPath, scriptsPath] of SURFACES) { const content = fs.readFileSync(path.join(DIST, relPath), 'utf-8'); - expect(content).toContain(`node ${scriptsPath}/embed-prompt.mjs`); + expect(content).toContain(`${scriptsPath}/impeccable embed-prompt`); expect(content).not.toContain('{{scripts_path}}'); } }); @@ -703,7 +676,7 @@ describe('agent bodies resolve placeholders on every surface that ships them', ( name: 'impeccable-synthetic', codexName: 'impeccable_synthetic', description: 'synthetic agent', - body: 'Run `node {{scripts_path}}/embed-prompt.mjs` and ask {{model}}. ', + body: 'Run `{{scripts_path}}/impeccable embed-prompt` and ask {{model}}. ', }, ], }; @@ -725,7 +698,7 @@ describe('agent bodies resolve placeholders on every surface that ships them', ( path.join(synthDist, 'codex/.codex/skills/impeccable/agents/impeccable_synthetic.toml'), 'utf-8' ); - expect(codexToml).toContain('node .codex/skills/impeccable/scripts/embed-prompt.mjs'); + expect(codexToml).toContain('.codex/skills/impeccable/scripts/impeccable embed-prompt'); // The model name belongs to PROVIDER_PLACEHOLDERS and may change; what // this pins is that {{model}} resolved to something. expect(codexToml).toMatch(/and ask \S+\./); diff --git a/tests/hook-build.test.mjs b/tests/hook-build.test.mjs index 11ad34cbb..8fee374ad 100644 --- a/tests/hook-build.test.mjs +++ b/tests/hook-build.test.mjs @@ -7,7 +7,7 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; -import { fileURLToPath, pathToFileURL } from 'node:url'; +import { fileURLToPath } from 'node:url'; import { buildClaudeSettingsManifest, @@ -26,30 +26,29 @@ function readJson(rel) { return JSON.parse(fs.readFileSync(path.join(REPO_ROOT, rel), 'utf8')); } -// The runtime probe every hook command must carry (issue #410): a node below -// the engines floor exits the command at 0 instead of dying on ESM parse. The -// expected floor comes from package.json engines, so probe and contract cannot -// drift apart. -const ENGINES_NODE_MAJOR = parseInt( - JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'package.json'), 'utf8')).engines.node.replace(/[^\d.]/g, ''), - 10, -); -const NODE_PROBE = `process.exit(Math.min(parseInt(process.versions.node,10),${ENGINES_NODE_MAJOR})===${ENGINES_NODE_MAJOR}?0:1)`; - -function expectCommand(command, expectedPath) { +// Every hook command is the launcher shipped in the skill's scripts dir, +// invoked as `/impeccable ` behind an existence guard: a +// missing launcher exits 0 (issue #399: user-level manifests fire in every +// project) and a present one keeps its own exit code, so Claude's exit-2 +// blocking signal still reaches the agent. No runtime probe: the launcher +// runs a self-contained binary, so there is no Node on the path to check. +function expectCommand(command, expectedScriptsDir, verb = 'hook') { assert.equal(typeof command, 'string'); - // node-command providers carry the missing-file guard (issue #399: exits 0 - // when absent, preserves node's exit code when present) plus the runtime - // probe. GitHub's portable `$(git rev-parse)` form is guarded too, so it - // lands in the same branch. - if (command.startsWith('[ ! -f "')) { - assert.match(command, /\|\| node "/); - assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`); - } else { - assert.match(command, /^node "|^bash -c|\$\(git rev-parse/); - } - assert.ok(command.includes(expectedPath), `missing ${expectedPath} in ${command}`); - assert.ok(!command.includes('hook-probe.mjs'), `probe hook still referenced in ${command}`); + const launcher = `${expectedScriptsDir}/impeccable`; + assert.ok(command.includes(launcher), `missing ${launcher} in ${command}`); + assert.ok( + command.endsWith(`"] || "`) === false && command.includes(`" ] || "`), + `missing existence guard in ${command}`, + ); + assert.match(command, new RegExp(`^\\[ ! -f "[^"]*/impeccable" \\] \\|\\| "[^"]*/impeccable" ${verb}$`)); + assert.ok(!command.includes('node '), `hook command must not depend on node: ${command}`); + assert.ok(!command.includes('.mjs'), `hook command still names a Node script: ${command}`); +} + +function expectWindowsCommand(command, expectedScriptsDir, verb = 'hook') { + assert.equal(typeof command, 'string'); + const launcher = `${expectedScriptsDir}/impeccable.cmd`; + assert.equal(command, `if exist "${launcher}" ("${launcher}" ${verb} & exit /b)`); } function manifestCommands(manifest) { @@ -77,7 +76,7 @@ describe('hook manifest builders', () => { assert.equal(handler.type, 'command'); assert.equal(handler.timeout, 5); assert.equal(handler.statusMessage, 'Checking UI changes'); - expectCommand(handler.command, '.claude/skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, '.claude/skills/impeccable/scripts'); assert.ok(handler.command.includes('${CLAUDE_PROJECT_DIR}')); assert.equal(handler.args, undefined); assert.equal(manifest.hooks.SessionStart, undefined); @@ -87,7 +86,7 @@ describe('hook manifest builders', () => { assert.equal(manifest.hooks.Stop[0].matcher, undefined); assert.equal(stop.timeout, 30); assert.equal(stop.statusMessage, 'Design deep pass'); - expectCommand(stop.command, '.claude/skills/impeccable/scripts/hook.mjs'); + expectCommand(stop.command, '.claude/skills/impeccable/scripts'); }); it('builds Codex project-local hooks for the real detector hook', () => { @@ -103,7 +102,7 @@ describe('hook manifest builders', () => { assert.equal(handler.type, 'command'); assert.equal(handler.timeout, 5); assert.equal(handler.statusMessage, 'Checking UI changes'); - expectCommand(handler.command, '.codex/skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, '.codex/skills/impeccable/scripts'); assert.ok(!handler.command.includes('git rev-parse --show-toplevel')); assert.ok(!handler.command.includes('${PLUGIN_ROOT}')); assert.equal(manifest.hooks.SessionStart, undefined); @@ -112,7 +111,12 @@ describe('hook manifest builders', () => { // pass too. const stop = manifest.hooks.Stop[0].hooks[0]; assert.equal(stop.timeout, 30); - expectCommand(stop.command, '.codex/skills/impeccable/scripts/hook.mjs'); + expectCommand(stop.command, '.codex/skills/impeccable/scripts'); + + // Codex 0.146.0+ selects `commandWindows` on Windows (issue #452), where + // the POSIX guard is not a command; that form calls impeccable.cmd. + expectWindowsCommand(handler.commandWindows, '.codex/skills/impeccable/scripts'); + expectWindowsCommand(stop.commandWindows, '.codex/skills/impeccable/scripts'); }); it('derives the Codex hook payload path from the install dir', () => { @@ -120,22 +124,22 @@ describe('hook manifest builders', () => { // `.codex`-directory install at `.codex/skills`, a `.agents` (Codex repo // skills) install at `.agents/skills`. const codexDir = buildCodexHooksManifest('.codex'); - expectCommand(codexDir.hooks.PostToolUse[0].hooks[0].command, '.codex/skills/impeccable/scripts/hook.mjs'); - expectCommand(codexDir.hooks.Stop[0].hooks[0].command, '.codex/skills/impeccable/scripts/hook.mjs'); + expectCommand(codexDir.hooks.PostToolUse[0].hooks[0].command, '.codex/skills/impeccable/scripts'); + expectCommand(codexDir.hooks.Stop[0].hooks[0].command, '.codex/skills/impeccable/scripts'); const agentsDir = buildCodexHooksManifest('.agents'); - expectCommand(agentsDir.hooks.PostToolUse[0].hooks[0].command, '.agents/skills/impeccable/scripts/hook.mjs'); - expectCommand(agentsDir.hooks.Stop[0].hooks[0].command, '.agents/skills/impeccable/scripts/hook.mjs'); + expectCommand(agentsDir.hooks.PostToolUse[0].hooks[0].command, '.agents/skills/impeccable/scripts'); + expectCommand(agentsDir.hooks.Stop[0].hooks[0].command, '.agents/skills/impeccable/scripts'); assert.ok(!agentsDir.hooks.PostToolUse[0].hooks[0].command.includes('.codex/skills')); // hooksJsonFor threads the provider's configDir through to the builder. expectCommand( hooksJsonFor('codex', { configDir: '.agents' }).hooks.PostToolUse[0].hooks[0].command, - '.agents/skills/impeccable/scripts/hook.mjs', + '.agents/skills/impeccable/scripts', ); expectCommand( hooksJsonFor('codex').hooks.PostToolUse[0].hooks[0].command, - '.codex/skills/impeccable/scripts/hook.mjs', + '.codex/skills/impeccable/scripts', ); }); @@ -149,7 +153,7 @@ describe('hook manifest builders', () => { assert.equal(manifest.hooks.afterFileEdit, undefined); assert.equal(manifest.hooks.stop, undefined); assert.equal(manifest.hooks.sessionStart, undefined); - expectCommand(beforeEdit.command, '.cursor/skills/impeccable/scripts/hook-before-edit.mjs'); + expectCommand(beforeEdit.command, '.cursor/skills/impeccable/scripts', 'hook-before-edit'); assert.equal(beforeEdit.timeout, 5); }); @@ -166,7 +170,7 @@ describe('hook manifest builders', () => { assert.equal(entry.timeoutSec, 5); assert.equal(entry.timeout, undefined); assert.equal(entry.command, undefined); - expectCommand(entry.bash, '.github/skills/impeccable/scripts/hook.mjs'); + expectCommand(entry.bash, '.github/skills/impeccable/scripts'); assert.ok(entry.bash.includes('git rev-parse --show-toplevel')); assert.equal(manifest.hooks.PostToolUse, undefined); assert.equal(manifest.hooks.preToolUse, undefined); @@ -182,7 +186,7 @@ describe('hook manifest builders', () => { assert.equal(handler.type, 'command'); assert.equal(handler.timeout, 5); assert.equal(handler.statusMessage, 'Checking UI changes'); - expectCommand(handler.command, '.grok/skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, '.grok/skills/impeccable/scripts'); assert.ok(!handler.command.includes('${CLAUDE_PROJECT_DIR}')); assert.ok(!handler.command.includes('${GROK_PLUGIN_ROOT}')); assert.equal(manifest.hooks.SessionStart, undefined); @@ -190,51 +194,51 @@ describe('hook manifest builders', () => { const stop = manifest.hooks.Stop[0].hooks[0]; assert.equal(stop.timeout, 30); assert.equal(stop.statusMessage, 'Design deep pass'); - expectCommand(stop.command, '.grok/skills/impeccable/scripts/hook.mjs'); + expectCommand(stop.command, '.grok/skills/impeccable/scripts'); }); - it('probes the node runtime everywhere, and notices only where a channel exists', () => { - // Claude Code and Codex render a `systemMessage` from hook stdout, so their - // manifests carry the one-time unsupported-runtime notice. Cursor (output is - // permission-shaped; a message would block the edit), Grok (stdout ignored), - // and Copilot (contract unconfirmed) get the silent probe only. - const withNotice = [ + it('emits commandWindows only for Codex-shaped manifests', () => { + // Codex reads a `commandWindows` sibling; Claude, Cursor, Grok, and Copilot + // have no per-platform field, and an unknown key is a risk under a strict + // parser, so it stays off everywhere else. + const withWindows = [buildCodexHooksManifest(), buildCodexPluginHooksManifest()]; + const without = [ buildClaudeSettingsManifest(), buildClaudePluginHooksManifest(), - buildCodexHooksManifest(), - buildCodexPluginHooksManifest(), - ]; - const probeOnly = [ buildCursorHooksManifest(), buildGitHubHooksManifest(), buildGrokHooksManifest(), ]; - for (const manifest of [...withNotice, ...probeOnly]) { - for (const command of manifestCommands(manifest)) { - assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`); + const entries = (manifest) => { + const out = []; + const walk = (value) => { + if (Array.isArray(value)) { value.forEach(walk); return; } + if (value && typeof value === 'object') { + if (typeof value.command === 'string' || typeof value.bash === 'string') out.push(value); + Object.values(value).forEach(walk); + } + }; + walk(manifest.hooks); + return out; + }; + for (const manifest of withWindows) { + for (const entry of entries(manifest)) { + assert.equal(typeof entry.commandWindows, 'string', `missing commandWindows in ${JSON.stringify(entry)}`); + assert.ok(entry.commandWindows.includes('impeccable.cmd')); } } - for (const manifest of withNotice) { - for (const command of manifestCommands(manifest)) { - assert.ok(command.includes('systemMessage'), `missing notice in ${command}`); - assert.ok(command.includes('node-unsupported'), `missing once-only marker in ${command}`); + for (const manifest of without) { + for (const entry of entries(manifest)) { + assert.equal(entry.commandWindows, undefined, `unexpected commandWindows in ${JSON.stringify(entry)}`); } } - for (const manifest of probeOnly) { + for (const manifest of [...withWindows, ...without]) { for (const command of manifestCommands(manifest)) { - assert.ok(!command.includes('systemMessage'), `unexpected notice in ${command}`); + assert.ok(!/node|systemMessage|node-unsupported/.test(command), `Node-era fragment in ${command}`); } } }); - // Volta's Windows shims exec through `cmd /C`, which claims `<`, `>`, and - // newlines from the `node -e` payload, so the probe died before node ran and - // the guard read that as a missing runtime (volta-cli/volta#1791). Every - // command is asserted to carry NODE_PROBE above, so this covers them all. - it('keeps the runtime probe free of characters cmd.exe re-parses', () => { - assert.ok(!/[<>\n]/.test(NODE_PROBE), `cmd.exe-unsafe character in probe: ${NODE_PROBE}`); - }); - it('routes supported hook builders and leaves other providers alone', () => { assert.ok(hooksJsonFor('claude')); assert.ok(hooksJsonFor('codex')); @@ -245,7 +249,14 @@ describe('hook manifest builders', () => { }); }); -describe('generated hook artifacts in repo', () => { +// The tracked provider outputs are regenerated on main by the sync workflow +// (`bun run build:release`), never in a feature PR. Until that sync lands after +// the launcher swap, the tracked manifests still describe the Node scripts; +// gate these assertions on the synced launcher so a source-first branch is +// not red for output it is not allowed to stage. +const SYNCED = fs.existsSync(path.join(REPO_ROOT, '.claude/skills/impeccable/scripts/impeccable')); + +describe('generated hook artifacts in repo', { skip: SYNCED ? false : 'generated provider output not yet synced (bun run build:release on main)' }, () => { for (const rel of [ '.claude/settings.json', '.cursor/hooks.json', @@ -266,14 +277,12 @@ describe('generated hook artifacts in repo', () => { assert.deepEqual(readJson('.github/hooks/impeccable.json'), buildGitHubHooksManifest()); }); - it('Claude project settings reference hook.mjs in .claude/skills', () => { + it('Claude project settings reference the launcher in .claude/skills', () => { const manifest = readJson('.claude/settings.json'); const handler = manifest.hooks.PostToolUse[0].hooks[0]; - expectCommand(handler.command, '.claude/skills/impeccable/scripts/hook.mjs'); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.claude/skills/impeccable/scripts/hook.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.claude/skills/impeccable/scripts/hook-lib.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.claude/skills/impeccable/scripts/detector/detect-antipatterns.mjs'))); + expectCommand(handler.command, '.claude/skills/impeccable/scripts'); + assert.ok(fs.existsSync(path.join(REPO_ROOT, '.claude/skills/impeccable/scripts'))); }); it('Cursor project hooks reference only the pre-write runtime in .cursor/skills', () => { @@ -281,15 +290,12 @@ describe('generated hook artifacts in repo', () => { const beforeEdit = manifest.hooks.preToolUse[0]; assert.equal(Object.keys(manifest.hooks).length, 1); - expectCommand(beforeEdit.command, '.cursor/skills/impeccable/scripts/hook-before-edit.mjs'); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/hook-before-edit.mjs'))); - assert.equal(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/hook-after-edit.mjs')), false); - assert.equal(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/hook-stop.mjs')), false); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/hook-lib.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/detector/detect-antipatterns.mjs'))); + expectCommand(beforeEdit.command, '.cursor/skills/impeccable/scripts', 'hook-before-edit'); + assert.ok(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/impeccable'))); + assert.equal(fs.existsSync(path.join(REPO_ROOT, '.cursor/skills/impeccable/scripts/hook-before-edit.mjs')), false); }); - it('Codex project hooks reference hook.mjs in the .codex skill payload', () => { + it('Codex project hooks reference the launcher in the .codex skill payload', () => { // The committed `.codex/hooks.json` is the distribution artifact for a // `.codex`-directory install, whose skill payload lives at `.codex/skills/` // (issue: it previously hardcoded `.agents/skills`, so the guarded hook @@ -298,7 +304,7 @@ describe('generated hook artifacts in repo', () => { const manifest = readJson('.codex/hooks.json'); const handler = manifest.hooks.PostToolUse[0].hooks[0]; - expectCommand(handler.command, '.codex/skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, '.codex/skills/impeccable/scripts'); assert.ok(!handler.command.includes('.agents/skills')); // The self-consistent Codex bundle at `dist/codex/.codex/skills/` is a build @@ -309,21 +315,17 @@ describe('generated hook artifacts in repo', () => { // The repo ships the Codex skill payload at `.agents/skills` (the // layout CLI installs use, and where the rewritten command resolves). assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/SKILL.md'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/scripts/hook.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/scripts/hook-lib.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/scripts/detector/detect-antipatterns.mjs'))); + assert.ok(fs.existsSync(path.join(REPO_ROOT, '.agents/skills/impeccable/scripts'))); }); - it('GitHub Copilot repo hooks reference hook.mjs in the .github skill payload', () => { + it('GitHub Copilot repo hooks reference the launcher in the .github skill payload', () => { const manifest = readJson('.github/hooks/impeccable.json'); const entry = manifest.hooks.postToolUse[0]; assert.equal(entry.matcher, 'edit|create|apply_patch'); - expectCommand(entry.bash, '.github/skills/impeccable/scripts/hook.mjs'); + expectCommand(entry.bash, '.github/skills/impeccable/scripts'); assert.ok(fs.existsSync(path.join(REPO_ROOT, '.github/skills/impeccable/SKILL.md'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.github/skills/impeccable/scripts/hook.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.github/skills/impeccable/scripts/hook-lib.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, '.github/skills/impeccable/scripts/detector/detect-antipatterns.mjs'))); + assert.ok(fs.existsSync(path.join(REPO_ROOT, '.github/skills/impeccable/scripts'))); }); it('does not generate probe scripts into provider skill payloads', () => { @@ -358,7 +360,7 @@ describe('generated hook artifacts in repo', () => { const handler = manifest.hooks.PostToolUse[0].hooks[0]; assert.equal(manifest.hooks.PostToolUse[0].matcher, 'Edit|Write'); - expectCommand(handler.command, 'skills/impeccable/scripts/hook.mjs'); + expectCommand(handler.command, 'skills/impeccable/scripts'); // Resolves relative to the installed plugin, not a `.claude/skills/` layout. assert.ok(handler.command.includes('${CLAUDE_PLUGIN_ROOT}'), `plugin hook command must use $\{CLAUDE_PLUGIN_ROOT}: ${handler.command}`); @@ -368,24 +370,14 @@ describe('generated hook artifacts in repo', () => { // Stop deep pass ships in the plugin manifest too, plugin-root-relative. const stop = manifest.hooks.Stop[0].hooks[0]; assert.equal(stop.timeout, 30); - expectCommand(stop.command, 'skills/impeccable/scripts/hook.mjs'); + expectCommand(stop.command, 'skills/impeccable/scripts'); assert.ok(stop.command.includes('${CLAUDE_PLUGIN_ROOT}')); // The script the plugin hook points at must ship inside the plugin payload. - assert.ok(fs.existsSync(path.join(REPO_ROOT, 'plugin/skills/impeccable/scripts/hook.mjs'))); - assert.ok(fs.existsSync(path.join(REPO_ROOT, 'plugin/skills/impeccable/scripts/hook-lib.mjs'))); + assert.ok(fs.existsSync(path.join(REPO_ROOT, 'plugin/skills/impeccable/scripts'))); }); - it('keeps the marketplace hook repair matcher aligned with Claude Code', () => { - const hookAdmin = fs.readFileSync( - path.join(REPO_ROOT, 'plugin/skills/impeccable/scripts/hook-admin.mjs'), - 'utf8', - ); - assert.match(hookAdmin, /matcher: 'Edit\|Write'/); - assert.doesNotMatch(hookAdmin, /matcher: 'Edit\|Write\|MultiEdit'/); - }); - - it('generated hook runtime can import the bundled detector', async () => { + it('generated skill payloads ship the executable launcher and no Node scripts', () => { for (const scriptDir of [ '.claude/skills/impeccable/scripts', '.cursor/skills/impeccable/scripts', @@ -393,11 +385,16 @@ describe('generated hook artifacts in repo', () => { 'plugin/skills/impeccable/scripts', ]) { const abs = path.join(REPO_ROOT, scriptDir); - assert.ok(fs.existsSync(path.join(abs, 'detector', 'detect-antipatterns.mjs')), - `detector bundle missing in ${scriptDir}`); - const hookLib = await import(pathToFileURL(path.join(abs, 'hook-lib.mjs'))); - const detector = await hookLib.loadDetector(); - assert.equal(typeof detector.detectText, 'function'); + const launcher = path.join(abs, 'impeccable'); + assert.ok(fs.existsSync(launcher), `launcher missing in ${scriptDir}`); + if (process.platform !== 'win32') { + assert.ok(fs.statSync(launcher).mode & 0o111, `launcher not executable in ${scriptDir}`); + } + assert.ok(fs.existsSync(path.join(abs, 'impeccable.cmd')), `impeccable.cmd missing in ${scriptDir}`); + assert.ok(fs.existsSync(path.join(abs, 'VERSION')), `VERSION missing in ${scriptDir}`); + assert.equal(fs.existsSync(path.join(abs, 'bin')), false, `${scriptDir} must stay launcher-only in git; binaries ship only in release zips`); + const stray = fs.readdirSync(abs).filter((f) => f.endsWith('.mjs') || f === 'detector' || f === 'lib'); + assert.deepEqual(stray, [], `Node-era files still in ${scriptDir}`); } }); }); diff --git a/tests/lib/detector-bundle.test.js b/tests/lib/detector-bundle.test.js deleted file mode 100644 index d09c78dde..000000000 --- a/tests/lib/detector-bundle.test.js +++ /dev/null @@ -1,27 +0,0 @@ -import { describe, expect, test } from 'bun:test'; -import fs from 'fs'; -import path from 'path'; -import { readSourceFiles } from '../../scripts/lib/utils.js'; - -const ROOT = process.cwd(); - -describe('skill detector bundle', () => { - test('adds the detector wrapper and engine files to skill scripts', () => { - const { skills } = readSourceFiles(ROOT); - const skill = skills.find(s => s.name === 'impeccable'); - const scriptNames = new Set(skill.scripts.map(s => s.name)); - - expect(scriptNames.has('detect.mjs')).toBe(true); - expect(scriptNames.has('detector/detect-antipatterns.mjs')).toBe(true); - expect(scriptNames.has('detector/detect-antipatterns-browser.js')).toBe(true); - expect(scriptNames.has('detector/cli/main.mjs')).toBe(true); - expect(scriptNames.has('detector/engines/static-html/detect-html.mjs')).toBe(true); - }); - - test('critique references the bundled detector command', () => { - const critique = fs.readFileSync(path.join(ROOT, 'skill/reference/critique.md'), 'utf-8'); - - expect(critique).toContain('node {{scripts_path}}/detect.mjs --json [target]'); - expect(critique).not.toContain('npx impeccable detect'); - }); -}); diff --git a/tests/lib/impeccable-config.test.js b/tests/lib/impeccable-config.test.js deleted file mode 100644 index 8ca7aeaa3..000000000 --- a/tests/lib/impeccable-config.test.js +++ /dev/null @@ -1,301 +0,0 @@ -import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; -import { mkdtempSync, rmSync, mkdirSync, writeFileSync, readFileSync, existsSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { execFileSync } from 'node:child_process'; - -import { - extractFindingIgnoreValue, - filterDetectionFindings, - getHookConsent, - setHookConsent, - getLocalConfigPath, - getConfigPath, - ensureConfigGitExclude, - readDetectionConfig, - readRawDetectionConfig, - shouldIgnoreDetectionFile, - writeDetectionConfig, -} from '../../cli/lib/impeccable-config.mjs'; - -describe('cli/lib/impeccable-config', () => { - let root; - beforeEach(() => { root = mkdtempSync(join(tmpdir(), 'imp-cfg-')); }); - afterEach(() => rmSync(root, { recursive: true, force: true })); - - test('getHookConsent is undefined until a decision is recorded, then round-trips', () => { - expect(getHookConsent(root)).toBeUndefined(); - setHookConsent(root, 'declined'); - expect(getHookConsent(root)).toBe('declined'); - setHookConsent(root, 'accepted'); - expect(getHookConsent(root)).toBe('accepted'); - }); - - test('setHookConsent preserves unrelated keys in config.local.json', () => { - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getLocalConfigPath(root), JSON.stringify({ updateCheck: false, hook: { quiet: true } })); - setHookConsent(root, 'declined'); - const raw = JSON.parse(readFileSync(getLocalConfigPath(root), 'utf-8')); - expect(raw.updateCheck).toBe(false); - expect(raw.hook.quiet).toBe(true); - expect(raw.hook.consent).toBe('declined'); - }); - - test('config.local.json (per-developer) overrides config.json for consent', () => { - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getConfigPath(root), JSON.stringify({ hook: { consent: 'accepted' } })); - writeFileSync(getLocalConfigPath(root), JSON.stringify({ hook: { consent: 'declined' } })); - expect(getHookConsent(root)).toBe('declined'); - }); - - test('malformed config is tolerated (no throw, undefined consent)', () => { - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getLocalConfigPath(root), '{ not json'); - expect(getHookConsent(root)).toBeUndefined(); - }); - - test('writing consent gitignores config.local.json via .git/info/exclude', () => { - execFileSync('git', ['init', '-q'], { cwd: root }); - setHookConsent(root, 'declined'); - const exclude = readFileSync(join(root, '.git', 'info', 'exclude'), 'utf-8'); - expect(exclude).toContain('.impeccable/config.local.json'); - // Idempotent: a second write does not duplicate the marker block. - ensureConfigGitExclude(root); - const again = readFileSync(join(root, '.git', 'info', 'exclude'), 'utf-8'); - expect((again.match(/impeccable-config-ignore-start/g) || []).length).toBe(1); - // It uses .git/info/exclude, not a tracked .gitignore. - expect(existsSync(join(root, '.gitignore'))).toBe(false); - }); - - test('ensureConfigGitExclude is a no-op outside a git repo', () => { - expect(ensureConfigGitExclude(root)).toBe(false); - }); - - test('readDetectionConfig merges shared and local detector filters', () => { - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getConfigPath(root), JSON.stringify({ - detector: { - ignoreRules: ['side-tab'], - ignoreFiles: ['src/legacy/**'], - ignoreValues: [ - { rule: 'overused-font', value: 'Avenir Next', reason: 'team default' }, - { rule: 'design-system-color', value: '*', files: ['src/demo.css'] }, - ], - designSystem: { enabled: false }, - }, - })); - writeFileSync(getLocalConfigPath(root), JSON.stringify({ - detector: { - ignoreRules: ['gradient-text'], - ignoreFiles: ['src/local/**'], - ignoreValues: [ - { rule: 'overused-font', value: 'Avenir Next', reason: 'local override' }, - { rule: 'bounce-easing', value: 'bounce-ball' }, - ], - designSystem: { enabled: true }, - }, - })); - - const cfg = readDetectionConfig(root); - expect(cfg.ignoreRules).toEqual(['side-tab', 'gradient-text']); - expect(cfg.ignoreFiles).toEqual(['src/legacy/**', 'src/local/**']); - expect(cfg.ignoreValues).toEqual([ - { rule: 'overused-font', value: 'avenir next', reason: 'local override' }, - { rule: 'design-system-color', value: '*', files: ['src/demo.css'] }, - { rule: 'bounce-easing', value: 'bounce-ball' }, - ]); - expect(cfg.designSystem).toEqual({ enabled: true }); - }); - - test('readDetectionConfig remains backward-compatible with legacy hook filters', () => { - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getConfigPath(root), JSON.stringify({ - hook: { - ignoreRules: ['side-tab'], - ignoreFiles: ['src/legacy/**'], - ignoreValues: [{ rule: 'overused-font', value: 'Avenir Next' }], - designSystem: { enabled: false }, - }, - })); - const cfg = readDetectionConfig(root); - expect(cfg.ignoreRules).toEqual(['side-tab']); - expect(cfg.ignoreFiles).toEqual(['src/legacy/**']); - expect(cfg.ignoreValues).toEqual([{ rule: 'overused-font', value: 'avenir next' }]); - expect(cfg.designSystem).toEqual({ enabled: false }); - }); - - test('writeDetectionConfig writes detector config and strips legacy hook filters', () => { - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getConfigPath(root), JSON.stringify({ - updateCheck: false, - hook: { - consent: 'accepted', - quiet: true, - ignoreRules: ['legacy-rule'], - ignoreFiles: ['legacy/**'], - ignoreValues: [{ rule: 'overused-font', value: 'Legacy' }], - }, - })); - - const config = readRawDetectionConfig(root); - config.ignoreRules.push('side-tab'); - config.ignoreFiles.push('src/legacy/**'); - writeDetectionConfig(root, config); - - const raw = JSON.parse(readFileSync(getConfigPath(root), 'utf-8')); - expect(raw.updateCheck).toBe(false); - expect(raw.hook).toEqual({ consent: 'accepted', quiet: true }); - expect(raw.detector.ignoreRules).toEqual(['legacy-rule', 'side-tab']); - expect(raw.detector.ignoreFiles).toEqual(['legacy/**', 'src/legacy/**']); - expect(raw.detector.ignoreValues).toEqual([{ rule: 'overused-font', value: 'legacy' }]); - expect(raw.detector.designSystem).toBeUndefined(); - }); - - test('writeDetectionConfig local ignores do not create an implicit design-system override', () => { - execFileSync('git', ['init', '-q'], { cwd: root }); - mkdirSync(join(root, '.impeccable'), { recursive: true }); - writeFileSync(getConfigPath(root), JSON.stringify({ - detector: { designSystem: { enabled: false } }, - })); - - const local = readRawDetectionConfig(root, { local: true }); - local.ignoreValues.push({ rule: 'overused-font', value: 'Inter' }); - writeDetectionConfig(root, local, { local: true }); - - const rawLocal = JSON.parse(readFileSync(getLocalConfigPath(root), 'utf-8')); - expect(rawLocal.detector.designSystem).toBeUndefined(); - expect(readDetectionConfig(root).designSystem).toEqual({ enabled: false }); - expect(readFileSync(join(root, '.git', 'info', 'exclude'), 'utf-8')).toContain('.impeccable/config.local.json'); - }); - - test('shouldIgnoreDetectionFile matches relative and absolute paths', () => { - const cfg = { ignoreFiles: ['src/legacy/**', '*.generated.tsx'] }; - expect(shouldIgnoreDetectionFile(join(root, 'src', 'legacy', 'Card.tsx'), root, cfg)).toBe(true); - expect(shouldIgnoreDetectionFile(join(root, 'src', 'Card.generated.tsx'), root, cfg)).toBe(true); - expect(shouldIgnoreDetectionFile(join(root, 'src', 'Card.tsx'), root, cfg)).toBe(false); - }); - - test('filterDetectionFindings matches hook ignore value semantics', () => { - const findings = [ - { antipattern: 'overused-font', file: join(root, 'src', 'main.css'), line: 1, snippet: 'Primary font: Avenir Next' }, - { antipattern: 'overused-font', file: join(root, 'src', 'other.css'), line: 2, snippet: 'Primary font: Karla' }, - { antipattern: 'design-system-color', file: join(root, 'src', 'demo.css'), line: 3, ignoreValue: '#8b5cf6' }, - { antipattern: 'design-system-color', file: join(root, 'src', 'real.css'), line: 4, ignoreValue: '#8b5cf6' }, - { antipattern: 'design-system-font', file: join(root, 'src', 'demo.css'), line: 5, ignoreValue: 'Avenir Next' }, - { antipattern: 'overused-font', file: join(root, 'src', 'fonts.css'), line: 6, snippet: 'Google Fonts: space grotesk' }, - ]; - const filtered = filterDetectionFindings(findings, { - ignoreRules: [], - ignoreValues: [ - { rule: 'overused-font', value: 'avenir next' }, - { rule: 'design-system-color', value: '*', files: ['src/demo.css'] }, - { rule: 'design-system-font', value: '*' }, - { rule: 'overused-font', value: 'space grotesk' }, - ], - }); - - expect(filtered.map((f) => `${f.antipattern}:${f.line}`)).toEqual([ - 'overused-font:2', - 'design-system-color:4', - 'design-system-font:5', - ]); - }); - - test('filterDetectionFindings honors file-scoped wildcard ignores for non-value-bearing rules', () => { - const findings = [ - { antipattern: 'side-tab', file: join(root, 'components', 'TopicCard.jsx'), line: 331, snippet: "borderLeft: '7px solid" }, - { antipattern: 'side-tab', file: join(root, 'components', 'Other.jsx'), line: 12, snippet: "borderLeft: '7px solid" }, - ]; - const filtered = filterDetectionFindings(findings, { - ignoreRules: [], - ignoreValues: [{ rule: 'side-tab', value: '*', files: ['**/TopicCard.jsx'] }], - }); - expect(filtered.map((f) => `${f.antipattern}:${f.line}`)).toEqual(['side-tab:12']); - }); - - test('filterDetectionFindings matches equivalent design-system color values', () => { - const findings = [ - { antipattern: 'design-system-color', file: join(root, 'src', 'rgb.css'), line: 1, ignoreValue: 'rgb(139, 92, 246)' }, - { antipattern: 'design-system-color', file: join(root, 'src', 'hex.css'), line: 2, ignoreValue: '#8b5cf6' }, - { antipattern: 'design-system-color', file: join(root, 'src', 'alpha.css'), line: 3, ignoreValue: 'rgba(139, 92, 246, 0.5)' }, - { antipattern: 'design-system-color', file: join(root, 'src', 'other.css'), line: 4, ignoreValue: '#8b5cf7' }, - { antipattern: 'design-system-radius', file: join(root, 'src', 'radius.css'), line: 5, ignoreValue: 'rgb(139, 92, 246)' }, - ]; - const filtered = filterDetectionFindings(findings, { - ignoreValues: [ - { rule: 'design-system-color', value: '#8b5cf6' }, - { rule: 'design-system-color', value: 'rgb(139 92 246 / 100%)' }, - ], - }); - - expect(filtered.map((f) => `${f.antipattern}:${f.line}`)).toEqual([ - 'design-system-color:3', - 'design-system-color:4', - 'design-system-radius:5', - ]); - }); - - test('filterDetectionFindings normalizes every supported CSS color unit', () => { - const findings = [ - { antipattern: 'design-system-color', line: 1, ignoreValue: '#f00' }, - { antipattern: 'design-system-color', line: 2, ignoreValue: 'rgb(100% 0% 0%)' }, - { antipattern: 'design-system-color', line: 3, ignoreValue: 'hsl(360deg 100% 50%)' }, - { antipattern: 'design-system-color', line: 4, ignoreValue: 'hsl(180deg 100% 50%)' }, - { antipattern: 'design-system-color', line: 5, ignoreValue: 'hsl(3.141592653589793rad 100% 50%)' }, - { antipattern: 'design-system-color', line: 6, ignoreValue: 'hsl(0.5turn 100% 50%)' }, - { antipattern: 'design-system-color', line: 7, ignoreValue: 'hsl(200grad 100% 50%)' }, - { antipattern: 'design-system-color', line: 8, ignoreValue: 'rgba(255, 0, 0, 0.5)' }, - { antipattern: 'design-system-color', line: 9, ignoreValue: 'rgb(100% 0% 0% / 50%)' }, - { antipattern: 'design-system-color', line: 10, ignoreValue: 'hsla(0, 100%, 50%, 50%)' }, - { antipattern: 'design-system-color', line: 11, ignoreValue: '#f008' }, - ]; - const filtered = filterDetectionFindings(findings, { - ignoreValues: [ - { rule: 'design-system-color', value: '#ff0000' }, - { rule: 'design-system-color', value: '#00ffff' }, - { rule: 'design-system-color', value: '#ff000080' }, - { rule: 'design-system-color', value: '#ff000088' }, - ], - }); - - expect(filtered).toEqual([]); - }); - - test('filterDetectionFindings rejects out-of-range and malformed CSS colors', () => { - const findings = [ - { antipattern: 'design-system-color', line: 1, ignoreValue: 'rgb(256 0 0)' }, - { antipattern: 'design-system-color', line: 2, ignoreValue: 'rgb(100.1% 0% 0%)' }, - { antipattern: 'design-system-color', line: 3, ignoreValue: 'rgba(255, 0, 0, 101%)' }, - { antipattern: 'design-system-color', line: 4, ignoreValue: 'rgba(255, 0, 0, -0.1)' }, - { antipattern: 'design-system-color', line: 5, ignoreValue: 'hsl(0 100 50%)' }, - { antipattern: 'design-system-color', line: 6, ignoreValue: 'hsl(0 101% 50%)' }, - { antipattern: 'design-system-color', line: 7, ignoreValue: 'hsl(0foo 100% 50%)' }, - { antipattern: 'design-system-color', line: 8, ignoreValue: '#ff00000' }, - ]; - const filtered = filterDetectionFindings(findings, { - ignoreValues: [ - { rule: 'design-system-color', value: '#ff0000' }, - { rule: 'design-system-color', value: '#ff000080' }, - ], - }); - - expect(filtered.map((finding) => finding.line)).toEqual([1, 2, 3, 4, 5, 6, 7, 8]); - }); - - test('extractFindingIgnoreValue handles fonts, Google font URLs, and motion snippets', () => { - expect(extractFindingIgnoreValue({ antipattern: 'overused-font', snippet: 'Primary font: Avenir Next (80% of text)' })).toBe('avenir next'); - expect(extractFindingIgnoreValue({ antipattern: 'overused-font', snippet: 'https://fonts.googleapis.com/css2?family=Alumni+Sans:wght@700' })).toBe('alumni sans'); - expect(extractFindingIgnoreValue({ antipattern: 'overused-font', snippet: 'Google Fonts: space grotesk' })).toBe('space grotesk'); - expect(extractFindingIgnoreValue({ antipattern: 'bounce-easing', snippet: 'animation: bounce-ball 1s infinite' })).toBe('bounce-ball'); - }); - - // This list is duplicated in skill/scripts/hook-lib.mjs. The two had drifted: - // font-size waivers worked in the hook but not in the CLI, so the same config - // filtered differently depending on which entry point read it. - test('extractFindingIgnoreValue covers design-system-font-size, matching the hook', () => { - expect(extractFindingIgnoreValue({ antipattern: 'design-system-font-size', ignoreValue: '0.82rem' })).toBe('0.82rem'); - expect(extractFindingIgnoreValue({ antipattern: 'design-system-radius', ignoreValue: '18px' })).toBe('18px'); - // A rule with no waivable value still extracts nothing. - expect(extractFindingIgnoreValue({ antipattern: 'side-tab', snippet: 'border-left: 4px' })).toBe(''); - }); -});