Fix: drop allowed-tools so skill activates in claude -p (#736)

Claude Code requires consent to activate skills that declare allowed-tools;
non-interactive hosts cannot provide it and the skill silently degrades.
Remove the frontmatter block from source and strip it in the plugin rewrite.

AI-assisted commit.

Co-authored-by: Cursor <cursoragent@cursor.com>

scripts/lib/plugin-paths.js

@@ -1,5 +1,6 @@
 import fs from 'fs';
 import path from 'path';
+import { generateYamlFrontmatter, parseFrontmatter } from './utils.js';
 
 /**
  * Rewrite project-relative script paths for the plugin subtree (issue #523).
@@ -26,15 +27,19 @@ export const CLAUDE_PROJECT_SCRIPTS_PATH = '.claude/skills/impeccable/scripts';
 
 export const PLUGIN_SCRIPTS_PATH = '<skill-base-dir>/scripts';
 
-// The project-path rule pre-approves a path inside the user's project, the
-// one place the plugin must NOT run scripts from. No replacement rule
-// exists: a wildcard pattern such as `node */skills/impeccable/scripts/*`
-// auto-approves any same-shaped path anywhere on disk, and frontmatter has
-// no variable bound to the loaded plugin root (CLAUDE_PLUGIN_ROOT is
-// hook-only). The plugin copy drops the rule and script runs go through
-// the normal Bash confirmation.
+// Claude Code requires user consent to activate a skill whose frontmatter
+// declares allowed-tools; non-interactive hosts (`claude -p`) cannot provide
+// it and the skill silently degrades (issue #736). The plugin copy drops the
+// key via structured frontmatter rewrite, not regex.
 export const PROJECT_ALLOWED_TOOLS_LINE = `  - Bash(${CLAUDE_PROJECT_SCRIPTS_PATH}/impeccable *)\n`;
 
+function stripAllowedToolsFrontmatter(content) {
+  const { frontmatter, body } = parseFrontmatter(content);
+  if (frontmatter['allowed-tools'] === undefined) return content;
+  delete frontmatter['allowed-tools'];
+  return `${generateYamlFrontmatter(frontmatter)}\n${body}`;
+}
+
 // Setup step 1's second sentence names the project path as the fallback
 // when the runtime reports no base directory. A plugin install has no
 // working project fallback (that path is the bug this rewrite exists to
@@ -72,10 +77,7 @@ export const AGENT_EMBED_FALLBACK =
  * unit suite can pin every rewrite without a build.
  */
 export function rewritePluginMarkdown(content) {
-  return content
-    // Order matters: the allowed-tools line contains the project path, so
-    // remove it before the generic path replacement rewrites it into a
-    // line the removal no longer matches.
+  return stripAllowedToolsFrontmatter(content)
     .replaceAll(PROJECT_ALLOWED_TOOLS_LINE, '')
     .replaceAll(SETUP_FALLBACK_TEXT, SETUP_PLUGIN_TEXT)
     .replaceAll(CLAUDE_PROJECT_SCRIPTS_PATH, PLUGIN_SCRIPTS_PATH)
@@ -154,10 +156,17 @@ export function verifyPluginSkillRewrite(skillMdPath) {
       'scripts/lib/plugin-paths.js (issue #523); update SETUP_FALLBACK_TEXT to the new wording.',
     );
   }
+  if (/^allowed-tools:/m.test(content)) {
+    throw new Error(
+      `Plugin rewrite drift: ${skillMdPath} still declares allowed-tools in frontmatter. ` +
+      'The plugin copy must drop the entire block so non-interactive sessions can activate ' +
+      'the skill (issue #736); update the removal in scripts/lib/plugin-paths.js.',
+    );
+  }
   if (/Bash\((?:node |[^)]*scripts\/impeccable)/.test(content)) {
     throw new Error(
       `Plugin rewrite drift: ${skillMdPath} still pre-approves an engine launcher or node script path. ` +
-      "SKILL.src.md's allowed-tools entry no longer matches the removal in " +
+      'A stray Bash(...) entry survived the allowed-tools removal in ' +
       'scripts/lib/plugin-paths.js (issue #523); the plugin ships no launcher pre-approval.',
     );
   }

skill/SKILL.src.md

@@ -3,9 +3,6 @@ name: impeccable
 description: "Use when the user wants to design, redesign, shape, critique, audit, polish, clarify, distill, harden, optimize, adapt, animate, colorize, extract, or otherwise improve a frontend interface. Covers websites, landing pages, dashboards, product UI, app shells, components, forms, settings, onboarding, and empty states. Handles UX review, visual hierarchy, information architecture, cognitive load, accessibility, performance, responsive behavior, theming, anti-patterns, typography, fonts, spacing, layout, alignment, color, motion, micro-interactions, UX copy, error states, edge cases, i18n, and reusable design systems or tokens. Also use for bland designs that need to become bolder or more delightful, loud designs that should become quieter, live browser iteration on UI elements, or ambitious visual effects that should feel technically extraordinary. Not for backend-only or non-UI tasks."
 argument-hint: "[{{command_hint}}] [target]"
 user-invocable: true
-allowed-tools:
-  - Bash(npx impeccable *)
-  - Bash({{scripts_path}}/impeccable *)
 license: Apache 2.0
 ---
 

tests/plugin-paths.test.js

@@ -15,6 +15,7 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
 import fs from 'fs';
 import os from 'os';
 import path from 'path';
+import { parseFrontmatter } from '../scripts/lib/utils.js';
 import {
   rewritePluginMarkdown,
   rewritePluginAgentMarkdown,
@@ -66,20 +67,24 @@ describe('rewritePluginMarkdown', () => {
     );
   });
 
-  test('removes the node pre-approval instead of widening it', () => {
-    const frontmatter = [
+  test('removes the entire allowed-tools frontmatter block', () => {
+    const input = [
+      '---',
+      'name: impeccable',
       'allowed-tools:',
       '  - Bash(npx impeccable *)',
       '  - Bash(.claude/skills/impeccable/scripts/impeccable *)',
+      'license: Apache 2.0',
       '---',
       '',
+      'Body text.',
     ].join('\n');
-    const output = rewritePluginMarkdown(frontmatter);
-    // The generic path rewrite alone would leave Bash(<skill-base-dir>/scripts/impeccable *),
-    // a dead literal, and any wildcard replacement would auto-approve
-    // same-shaped paths outside the plugin. The line must go entirely.
+    const output = rewritePluginMarkdown(input);
+    expect(output).not.toMatch(/^allowed-tools:/m);
     expect(output).not.toContain('scripts/impeccable *');
-    expect(output).toContain('  - Bash(npx impeccable *)\n---');
+    expect(output).not.toContain('npx impeccable');
+    expect(output).toContain('license: Apache 2.0');
+    expect(output).toContain('Body text.');
   });
 
   test('drops the project-path fallback clause from Setup step 1', () => {
@@ -256,9 +261,6 @@ describe('verifyPluginSkillRewrite', () => {
   };
 
   const goodSkill = [
-    'allowed-tools:',
-    '  - Bash(.claude/skills/impeccable/scripts/impeccable *)',
-    '',
     '1. Run `<skill-base-dir>/scripts/impeccable context` once per session, where `<skill-base-dir>` is the ' +
       "loaded base directory the runtime reports for this skill; keep cwd at the user's project. " +
       'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable <verb>` command in this skill ' +
@@ -269,6 +271,7 @@ describe('verifyPluginSkillRewrite', () => {
   test('accepts a correctly rewritten SKILL.md', () => {
     const p = writeSkill(rewritePluginMarkdown(goodSkill));
     expect(() => verifyPluginSkillRewrite(p)).not.toThrow();
+    expect(fs.readFileSync(p, 'utf-8')).not.toMatch(/^allowed-tools:/m);
   });
 
   test('fails the build when the Setup fallback sentence no longer matched', () => {
@@ -280,22 +283,24 @@ describe('verifyPluginSkillRewrite', () => {
   });
 
   test('fails the build when a launcher pre-approval survives the removal', () => {
-    const reworded = goodSkill.replace(
-      'Bash(.claude/skills/impeccable/scripts/impeccable *)',
-      'Bash(.claude/skills/impeccable/scripts/impeccable.cmd *)',
+    const p = writeSkill(
+      rewritePluginMarkdown(goodSkill) + '\n  - Bash(<skill-base-dir>/scripts/impeccable.cmd *)\n',
     );
-    const p = writeSkill(rewritePluginMarkdown(reworded));
     expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher/);
   });
 
-  test('fails the build when a legacy node pre-approval survives', () => {
-    // The Node-era line is gone from SKILL.src.md, but a copy that still
-    // carries one must fail the same way as a surviving launcher line.
+  test('fails the build when allowed-tools frontmatter survives the removal', () => {
     const p = writeSkill(
-      rewritePluginMarkdown(goodSkill).replace(
-        'allowed-tools:\n',
-        'allowed-tools:\n  - Bash(node <skill-base-dir>/scripts/*)\n',
-      ),
+      'allowed-tools:\n  - Bash(npx impeccable *)\n' + rewritePluginMarkdown(goodSkill),
+    );
+    expect(() => verifyPluginSkillRewrite(p)).toThrow(/allowed-tools/);
+  });
+
+  test('fails the build when a legacy node pre-approval survives', () => {
+    // A copy that still carries a node pre-approval must fail the same way as
+    // a surviving launcher line, even outside an allowed-tools block.
+    const p = writeSkill(
+      rewritePluginMarkdown(goodSkill) + '\n  - Bash(node <skill-base-dir>/scripts/*)\n',
     );
     expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher or node script path/);
   });
@@ -310,3 +315,14 @@ describe('verifyPluginSkillRewrite', () => {
     expect(() => verifyPluginSkillRewrite(p)).toThrow(/still contains the project-relative scripts path/);
   });
 });
+
+describe('SKILL.src.md frontmatter', () => {
+  test('does not declare allowed-tools (issue #736)', () => {
+    const src = fs.readFileSync(
+      path.join(import.meta.dirname, '../skill/SKILL.src.md'),
+      'utf-8',
+    );
+    const { frontmatter } = parseFrontmatter(src);
+    expect(frontmatter['allowed-tools']).toBeUndefined();
+  });
+});