Add automatic design hook install and exceptions (#170)

* docs: add PRD for design detector hook integration Plans a PostToolUse hook for Claude Code and Codex that runs the existing design detector after every relevant file write and feeds findings back to the agent as advisory system-reminder context. No implementation in this commit; covers UX, technical design, build pipeline changes, distribution, coverage tradeoffs, and rollout. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: revise hook PRD with best-practices review Folds in the P0/P1/P2 findings from an online best-practices critique against the official Claude Code and Codex hook references plus 10+ 2026 community guides and similar prior-art tools (claw-hooks, claude-code-hooks-mastery). Key changes: - Exec form everywhere (Codex snippet was shell form), with Windows rationale. - Default timeout dropped from 10s to 5s. - Re-entrancy guard (CLAUDE_HOOK_DEPTH) and per-file edit counter. - Session-scoped finding dedup promoted from open question to v1. - Per-language inline-ignore syntax map (HTML/JSX/CSS/JS). - Hard-skip rules for sensitive paths and generated/lock files. - Honest framing about Claude Code lacking per-plugin hook disable. - Honest framing about Bash-written files being invisible in v1. - Codex Windows-not-supported call-out, feature flag note, trust ceremony detail. - Optional NDJSON audit log via IMPECCABLE_HOOK_LOG. - Findings cap lowered 8 → 5 with attention-budget rationale. - Versioned envelope ([impeccable@1]) on rendered template. - Expanded test plan, decision log, and stdin payload appendix. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(hooks): ship the design detector hook for Claude Code and Codex Implements docs/hooks-prd.md: a PostToolUse hook that runs the impeccable design detector after every Edit/Write/MultiEdit on a UI file and pushes findings into the agent's next-turn context as a short system reminder. Silent on clean files. Never blocks an edit. Why this matters: today, design slop (side-tab borders, gradient text, purple/cyan palettes, bounce easing, etc.) only gets caught when a human notices or someone explicitly runs /impeccable audit. The hook closes the loop at the moment slop is written. What ships in v1 - skill/scripts/hook.mjs: PostToolUse entry. Reads stdin, runs the detector in-process (no `npx impeccable` cold start), emits hookSpecificOutput.additionalContext when fresh findings exist. - skill/scripts/hook-lib.mjs: extracted helpers (config, cache, filter, render, audit log, runHook orchestrator). 100% unit-testable. - skill/scripts/hook-session-start.mjs: SessionStart greeting, gated by a project-scannable probe + 30-day throttle. - skill/scripts/hook-admin.mjs: backs /impeccable hooks on/off/status/ignore-rule/ignore-file/reset. Hardening built in - Re-entrancy guard (IMPECCABLE_HOOK_DEPTH) so the hook can never recursively spawn itself. - Hard-skip regexes for sensitive paths (.env, .pem, id_rsa, secrets, credentials, .git) and generated/lock/build output. These fire before the file is even read; cannot be turned off via config. - Path-traversal check on the inbound file_path. - Session-scoped dedup keyed by (session, file, rule, line) so the same finding never lands in context twice. Prevents the ~12.5K wasted tokens per chatty session called out in the PRD. - Per-(session, file) edit counter with a one-shot suppression notice on the 7th edit, silent after. - Fail-open contract: every error path returns exit 0 with no stdout. Optional NDJSON audit log via IMPECCABLE_HOOK_LOG. Three kill switches (precedence high to low): 1. IMPECCABLE_HOOK_DISABLED env var (1/true/yes/on, case-insensitive) 2. .impeccable/hook.json `enabled: false` 3. /impeccable hooks off slash command (writes the JSON) Inline ignores are language-aware. `// impeccable: ignore <rule>` for JS/TS, `` for HTML/Vue/Svelte/Astro, `{/* impeccable: ignore <rule> */}` for JSX/TSX, `/* impeccable: ignore <rule> */` for CSS. `*` matches any rule. Directive applies to the next non-blank line. Same shape as ESLint, Stylelint, Biome. Build pipeline - scripts/lib/transformers/hooks.js: per-provider hooks.json builders, plus the slim .codex-plugin/plugin.json manifest. - providers.js: emitHooks: 'claude' for claude-code, emitHooks: 'codex' for codex and agents. Codex also emits emitCodexPlugin. - factory.js: emits hooks/hooks.json next to the skills tree. - build.js: syncs hooks/ into harness roots and into the slim plugin/ subtree; writes .codex-plugin/plugin.json. Build is idempotent (verified: 98 staged files unchanged across two runs). Claude Code wiring uses exec form (command + args) and the ${CLAUDE_PLUGIN_ROOT} placeholder. Matcher: Edit|Write|MultiEdit. `if:` glob filters to UI extensions before spawning Node. PostToolUse timeout 5s, SessionStart timeout 3s. Codex wiring uses ${PLUGIN_ROOT} (Codex's native placeholder), matcher Edit|Write|apply_patch, no `if:` analog (the script does the extension filter). macOS and Linux only; hooks are disabled on Windows in current Codex builds. The trust ceremony and feature flag are documented in README.md. Routing - /impeccable hooks lives outside the 23-command router table on purpose: it is plumbing, not a design skill. The hidden routing slot is added to SKILL.md alongside pin/unpin so the LLM knows to dispatch it. The 23-command count and all stale-count validators remain happy. Tests - tests/hook.test.mjs: 38 unit tests covering env parsing, config load + defaults + malformed, cache round-trip + GC, ignoreRules/minSeverity/inline ignores (all four languages), globbing with **/*/{a,b}, render template with cap + clamp + 0-line prefix drop, audit log NDJSON, payload event-name parameterization, re-entrancy, kill switches, sensitive-path + generated-path + traversal skips, allowlist filter, config ignoreFiles, edit counter cycle including the 7th-edit notice, MultiEdit and apply_patch payload shapes, detector throw swallow, malformed stdin, missing file race. - tests/hook-build.test.mjs: 18 integration tests covering hook manifest shape (matcher, timeouts, exec form, if: glob, placeholders), Codex differences (${PLUGIN_ROOT}, no if:, no SessionStart), Codex plugin manifest (no inline hooks field to avoid the duplicate-file error), routing across the hooksJsonFor table, and presence of all three committed artifacts plus the bundled detector the runtime relative-import path depends on. Full suite: 175 bun tests + 186 node tests, all green. Docs - README.md: new "Design hook" section explaining default behavior, per-project / global / inline disable paths, the JSON schema knobs, the audit log debug flag, and the slop / a11y coverage split. - HARNESSES.md: flips the `hooks` row for Codex from No -> Yes (Claude was already Yes), adds a per-harness hook-surface table with the manifest location and matcher each provider uses. Open questions from the PRD intentionally deferred to v2: Bash-write blind spot, effort-aware suppression, Stop-hook session summary, per-rule severity, async hook mode. None block v1. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix Codex hook scanning: apply_patch paths and co-located stylesheets Parse file targets from Codex apply_patch command bodies, co-scan imported and sibling CSS when UI components are edited, drop the git-sweep PostToolUse group, and align Codex SessionStart manifest and trust docs with the official hooks spec. Co-authored-by: Cursor <cursoragent@cursor.com> * Gitignore hook session cache and drop local test HTML Hook dedup/throttle state in .impeccable/hook.cache.json is per-project runtime data like other .impeccable/ sidecars. Remove an untracked bad-nested-flexbox scratch page from site/public/. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix Claude Code hook: drop Edit-only if filter so Write/MultiEdit fire Claude's if permission rule binds to one tool name, so Edit(*.{…}) never spawned the hook on Write or MultiEdit despite the matcher listing them. Extension filtering now lives in hook-lib on both Claude and Codex. Co-authored-by: Cursor <cursoragent@cursor.com> * Surface Cursor design findings via stop-hook followup Replace dropped postToolUse additional_context with afterFileEdit recording and a one-shot stop followup_message so anti-pattern nudges reach the agent. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix design hook packaging and scans * Fix Cursor hook pending bucket fallback * Fix Sass hook scan coverage * Fix Cursor hook review findings * Fix session start dead hook normalization * Fix hook config and relative scan paths * Remove SessionStart design hook * Remove redundant afterFileEdit normalization * Fix Cursor suppression and module style scans * Fix sensitive path hook filter * Fix disabled Cursor stop hook emission * Refresh hook harness artifacts * Fix Cursor hook manifest install * Add hook ignore-value support * Ignore hook runtime files locally * Fix Codex plugin hook packaging * fix: address PR review bot findings Block numeric hook depth counters from re-entering. Avoid following stylesheet imports from traversal-looking hook targets. * fix: gate ignore-value suggestions by supported rules Only render exact ignore-value commands when the same finding can be suppressed by ignoreValues. * Package Codex plugin as hook-only * Remove Codex plugin packaging * Recover hook install probe plumbing * Remove Codex hook packaging follow-up doc * Remove extra hook docs and skill wording changes * Install real design hooks via skills CLI * Add provider hook smoke runner * Fix Cursor hook delivery with preToolUse gate * Simplify Cursor hook install to preToolUse * Clarify confirmed hook exceptions * Persist hook ignores in shared config * Guard font hook exceptions * Fix hook install after main rebase * Fix hook scan target handling * fix: address hook review findings * Address hook review feedback * Stabilize DeepSeek insert live fixture * Fix Cursor hook Python shell write bypass --------- Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-17 00:26:41 +03:00 · 2026-06-13 21:19:19 -07:00
parent 92d6141cdf
commit 672517f76e
155 changed files with 32853 additions and 107 deletions
@@ -59,6 +59,26 @@ const GLOBAL_HARNESS_HINTS = [
 // Last-resort default when nothing is detected: Claude Code + the universal
 // (.agents, also Codex) folder, which covers the most common setups.
 const DEFAULT_TARGETS = ['.claude', '.agents'];
+const IMPECCABLE_HOOK_COMMAND_MARKERS = [
+  'skills/impeccable/scripts/hook-probe.mjs',
+  'skills/impeccable/scripts/hook.mjs',
+  'skills/impeccable/scripts/hook-before-edit.mjs',
+  'skills/impeccable/scripts/hook-after-edit.mjs',
+  'skills/impeccable/scripts/hook-stop.mjs',
+];
+const PROVIDER_HOOK_ARTIFACTS = {
+  '.claude': [
+    { sourceProvider: '.claude', rel: 'settings.json', destProvider: '.claude' },
+  ],
+  '.cursor': [
+    { sourceProvider: '.cursor', rel: 'hooks.json', destProvider: '.cursor' },
+  ],
+  // Codex reads skills from `.agents/skills`, but project hooks from
+  // `.codex/hooks.json`, so the `.agents` install target owns this sidecar.
+  '.agents': [
+    { sourceProvider: '.codex', rel: 'hooks.json', destProvider: '.codex' },
+  ],
+};

 function ask(question) {
  const rl = createInterface({ input: process.stdin, output: process.stdout });
@@ -392,26 +412,140 @@ function copyProviderSkills(bundleDir, root, targets) {
  let written = 0;
  for (const provider of targets) {
    const srcDir = join(bundleDir, provider, 'skills');
-    if (!existsSync(srcDir)) continue;
-    const localSkillsDir = join(root, provider, 'skills');
-    // A previous `npx skills` install may have left this provider's skills dir
-    // as a symlink to another provider's canonical copy. Drop the link so we
-    // write a real, provider-specific directory instead of writing through it.
-    try {
-      if (lstatSync(localSkillsDir).isSymbolicLink()) unlinkSync(localSkillsDir);
-    } catch {}
-    for (const skill of readdirSync(srcDir, { withFileTypes: true })) {
-      if (!skill.isDirectory()) continue;
-      const src = join(srcDir, skill.name);
-      const dest = join(localSkillsDir, skill.name);
-      rmSync(dest, { recursive: true, force: true });
-      copyDirSync(src, dest);
-      written++;
+    if (existsSync(srcDir)) {
+      const localSkillsDir = join(root, provider, 'skills');
+      // A previous `npx skills` install may have left this provider's skills dir
+      // as a symlink to another provider's canonical copy. Drop the link so we
+      // write a real, provider-specific directory instead of writing through it.
+      try {
+        if (lstatSync(localSkillsDir).isSymbolicLink()) unlinkSync(localSkillsDir);
+      } catch {}
+      for (const skill of readdirSync(srcDir, { withFileTypes: true })) {
+        if (!skill.isDirectory()) continue;
+        const src = join(srcDir, skill.name);
+        const dest = join(localSkillsDir, skill.name);
+        rmSync(dest, { recursive: true, force: true });
+        copyDirSync(src, dest);
+        written++;
+      }
    }
  }
  return written;
 }

+function hookArtifactsForProvider(bundleDir, root, provider) {
+  return (PROVIDER_HOOK_ARTIFACTS[provider] || []).map(({ sourceProvider, rel, destProvider }) => ({
+    src: join(bundleDir, sourceProvider, rel),
+    dest: join(root, destProvider, rel),
+  }));
+}
+
+function expectedHookDests(root, providers) {
+  const targets = Array.isArray(providers) ? providers : [providers];
+  return targets.flatMap(provider =>
+    (PROVIDER_HOOK_ARTIFACTS[provider] || []).map(({ rel, destProvider }) => join(root, destProvider, rel))
+  );
+}
+
+function valueHasImpeccableHookMarker(value) {
+  if (typeof value === 'string') {
+    return IMPECCABLE_HOOK_COMMAND_MARKERS.some(marker => value.includes(marker));
+  }
+  if (Array.isArray(value)) return value.some(valueHasImpeccableHookMarker);
+  if (value && typeof value === 'object') {
+    return Object.values(value).some(valueHasImpeccableHookMarker);
+  }
+  return false;
+}
+
+function stripImpeccableHookEntry(entry) {
+  if (!entry || typeof entry !== 'object') return entry;
+  if (valueHasImpeccableHookMarker(entry.command) || valueHasImpeccableHookMarker(entry.args)) {
+    return null;
+  }
+  if (!Array.isArray(entry.hooks)) return entry;
+
+  const strippedHooks = entry.hooks
+    .map(stripImpeccableHookEntry)
+    .filter(Boolean);
+
+  if (strippedHooks.length === 0 && entry.hooks.some(valueHasImpeccableHookMarker)) {
+    return null;
+  }
+
+  return { ...entry, hooks: strippedHooks };
+}
+
+function stripImpeccableHookEntries(entries) {
+  if (!Array.isArray(entries)) return [];
+  return entries
+    .map(stripImpeccableHookEntry)
+    .filter(Boolean);
+}
+
+function mergeHookManifests(existing, fresh) {
+  const existingObject = existing && typeof existing === 'object' && !Array.isArray(existing) ? existing : {};
+  const freshObject = fresh && typeof fresh === 'object' && !Array.isArray(fresh) ? fresh : {};
+  const existingHooks = existingObject.hooks && typeof existingObject.hooks === 'object' && !Array.isArray(existingObject.hooks)
+    ? existingObject.hooks
+    : {};
+  const freshHooks = freshObject.hooks && typeof freshObject.hooks === 'object' && !Array.isArray(freshObject.hooks)
+    ? freshObject.hooks
+    : {};
+
+  const merged = { ...existingObject, hooks: {} };
+  if (freshObject.version !== undefined) merged.version = freshObject.version;
+  if (freshObject.description !== undefined) merged.description = freshObject.description;
+
+  const hookEvents = new Set([...Object.keys(existingHooks), ...Object.keys(freshHooks)]);
+  for (const event of hookEvents) {
+    const preserved = stripImpeccableHookEntries(existingHooks[event]);
+    const added = Array.isArray(freshHooks[event]) ? freshHooks[event] : [];
+    const mergedEntries = [...preserved, ...added];
+    if (mergedEntries.length > 0) merged.hooks[event] = mergedEntries;
+  }
+
+  return merged;
+}
+
+function readJsonFile(filePath, description) {
+  try {
+    return JSON.parse(readFileSync(filePath, 'utf-8'));
+  } catch (e) {
+    throw new Error(`${description} is not valid JSON: ${filePath}. ${e.message}`);
+  }
+}
+
+function copyProviderHooks(bundleDir, root, providers, { force = false } = {}) {
+  const targets = Array.isArray(providers) ? providers : [providers];
+  const written = [];
+  for (const provider of targets) {
+    for (const { src, dest } of hookArtifactsForProvider(bundleDir, root, provider)) {
+      if (!existsSync(src)) continue;
+      const fresh = readJsonFile(src, 'Bundled hook manifest');
+      let next = fresh;
+
+      if (existsSync(dest)) {
+        try {
+          const existing = JSON.parse(readFileSync(dest, 'utf-8'));
+          next = mergeHookManifests(existing, fresh);
+        } catch {
+          if (!force) {
+            throw new Error(`Existing hook manifest is not valid JSON: ${dest}. Re-run with --force to replace it.`);
+          }
+          writeFileSync(`${dest}.bak`, readFileSync(dest));
+          next = fresh;
+        }
+      }
+
+      mkdirSync(dirname(dest), { recursive: true });
+      writeFileSync(dest, `${JSON.stringify(next, null, 2)}\n`);
+      written.push(provider);
+    }
+  }
+  return [...new Set(written)];
+}
+
 function resolveLinkSource(sourceValue, root) {
  const sourcePath = sourceValue || '.impeccable';
  const checkoutRoot = isAbsolute(sourcePath) ? sourcePath : resolve(root, sourcePath);
@@ -549,12 +683,30 @@ async function link(flags) {
 async function install(flags) {
  const force = flags.includes('--force');
  const yes = flags.includes('-y') || flags.includes('--yes');
+  const installHooks = !flags.includes('--no-hooks');
  const providersValue = getFlagValue(flags, '--providers');
  const root = findProjectRoot();
  const existing = isAlreadyInstalled(root);

  if (existing && !force) {
    console.log(`Impeccable skills are already installed (found in ${existing}/).`);
+    const targets = providersValue ? resolveInstallTargets(root, providersValue) : findInstalledProviders(root);
+    const missingHookDests = installHooks
+      ? expectedHookDests(root, targets).filter(dest => !existsSync(dest))
+      : [];
+    if (missingHookDests.length > 0) {
+      let bundleDir;
+      try {
+        bundleDir = await downloadAndExtractBundle();
+        const hookTargets = copyProviderHooks(bundleDir, root, targets);
+        if (hookTargets.length > 0) console.log(`Installed hooks into: ${hookTargets.join(', ')}`);
+      } catch (e) {
+        console.error(`Hook install failed: ${e.message}`);
+        process.exit(1);
+      } finally {
+        if (bundleDir) rmSync(bundleDir, { recursive: true, force: true });
+      }
+    }
    console.log('Run with --force to reinstall.\n');
    process.exit(0);
  }
@@ -594,8 +746,10 @@ async function install(flags) {
  migrateUnprefixImpeccable(root);

  let written = 0;
+  let hookTargets = [];
  try {
    written = copyProviderSkills(bundleDir, root, targets);
+    hookTargets = installHooks ? copyProviderHooks(bundleDir, root, targets, { force }) : [];
  } catch (e) {
    rmSync(bundleDir, { recursive: true, force: true });
    console.error(`Install failed: ${e.message}`);
@@ -608,6 +762,7 @@ async function install(flags) {
    process.exit(1);
  }
  console.log(`Installed impeccable into: ${targets.join(', ')}`);
+  if (hookTargets.length > 0) console.log(`Installed hooks into: ${hookTargets.join(', ')}`);

  console.log('\nDone! Run /impeccable init in your AI harness to set up design context.\n');
 }
@@ -692,6 +847,8 @@ function downloadFile(url, dest) {

 async function update(flags = []) {
  const yes = flags.includes('-y') || flags.includes('--yes');
+  const force = flags.includes('--force');
+  const installHooks = !flags.includes('--no-hooks');

  // Download the latest skills directly from impeccable.style.
  // We skip `npx skills update` because it has a known upstream bug
@@ -726,10 +883,19 @@ async function update(flags = []) {

  // Compare local vs remote -- skip if already up to date
  if (isUpToDate(root, copyProviders, tmpDir)) {
-    rmSync(tmpDir, { recursive: true, force: true });
-    const v = getSkillsVersion(root);
-    console.log(`Skills are up to date${v ? ` (v${v})` : ''}. Nothing to do.`);
-    process.exit(0);
+    try {
+      const hookTargets = installHooks ? copyProviderHooks(tmpDir, root, copyProviders, { force }) : [];
+      rmSync(tmpDir, { recursive: true, force: true });
+      const v = getSkillsVersion(root);
+      console.log(`Skills are up to date${v ? ` (v${v})` : ''}.`);
+      if (hookTargets.length > 0) console.log(`Installed hooks into: ${hookTargets.join(', ')}`);
+      console.log('Nothing else to do.');
+      process.exit(0);
+    } catch (e) {
+      rmSync(tmpDir, { recursive: true, force: true });
+      console.error(`Update failed: ${e.message}`);
+      process.exit(1);
+    }
  }

  console.log(`Found skills in: ${copyProviders.join(', ')}`);
@@ -769,11 +935,13 @@ async function update(flags = []) {
        updated++;
      }
    }
+    const hookTargets = installHooks ? copyProviderHooks(tmpDir, root, providers, { force }) : [];

    rmSync(tmpDir, { recursive: true, force: true });

    const v = getSkillsVersion(root);
    console.log(`Updated ${updated} skill(s)${v ? ` to v${v}` : ''}.`);
+    if (hookTargets.length > 0) console.log(`Installed hooks into: ${hookTargets.join(', ')}`);
    console.log('Done!\n');
  } catch (e) {
    console.error(`Update failed: ${e.message}`);
@@ -798,7 +966,16 @@ function copyDirSync(src, dest) {
 // ─── Test surface ───────────────────────────────────────────────────────────
 // Exported so the test suite exercises the real implementation rather than a
 // reimplementation in a helper script (which is how bugs slip through).
-export { migrateUnprefixImpeccable, linkProviderSkills, resolveLinkSource };
+export {
+  copyProviderHooks,
+  copyProviderSkills,
+  expectedHookDests,
+  linkProviderSkills,
+  mergeHookManifests,
+  migrateUnprefixImpeccable,
+  resolveInstallTargets,
+  resolveLinkSource,
+};

 // ─── Router ───────────────────────────────────────────────────────────────────

@@ -446,7 +446,7 @@ function detectText(content, filePath, options = {}) {

  // Run regex matchers on the full file content (catches Tailwind classes, inline styles)
  // Enable block context for CSS files where related properties span multiple lines
-  const cssLike = new Set(['.css', '.scss', '.less']);
+  const cssLike = new Set(['.css', '.scss', '.sass', '.less']);
  findings.push(...runRegexMatchers(lines, filePath, 0, cssLike.has(ext) || null, {
    profile,
    phase: 'source',
@@ -11,7 +11,7 @@ const SKIP_DIRS = new Set([
 ]);

 const SCANNABLE_EXTENSIONS = new Set([
-  '.html', '.htm', '.css', '.scss', '.less',
+  '.html', '.htm', '.css', '.scss', '.sass', '.less',
  '.jsx', '.tsx', '.js', '.ts',
  '.vue', '.svelte', '.astro',
 ]);