Fix: omit allowed-tools from Claude output only (#736)

Keep allowed-tools in shared source for other providers; drop it from the
Claude Code transformer and plugin rewrite so claude -p can activate the
skill. Verify gate now checks parsed frontmatter, not body text.

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
+// Claude Code requires user consent to activate a skill whose frontmatter
-// one place the plugin must NOT run scripts from. No replacement rule
+// declares allowed-tools; non-interactive hosts (`claude -p`) cannot provide
-// exists: a wildcard pattern such as `node */skills/impeccable/scripts/*`
+// it and the skill silently degrades (issue #736). The plugin copy drops the
-// auto-approves any same-shaped path anywhere on disk, and frontmatter has
+// key via structured frontmatter rewrite, not regex.
-// 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.
 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
+  return stripAllowedToolsFrontmatter(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.
     .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 (parseFrontmatter(content).frontmatter['allowed-tools'] !== undefined) {
+    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.',
     );
   }

scripts/lib/transformers/providers.js

@@ -29,7 +29,9 @@ export const PROVIDERS = {
     providerTags: ['claude-code', 'claude'],
     configDir: '.claude',
     displayName: 'Claude Code',
-    frontmatterFields: ['user-invocable', 'argument-hint', 'license', 'compatibility', 'metadata', 'allowed-tools'],
+    frontmatterFields: ['user-invocable', 'argument-hint', 'license', 'compatibility', 'metadata'],
+    // allowed-tools omitted: Claude Code blocks skill activation in non-interactive
+    // sessions when the field is present (issue #736). Other providers keep it.
     agentFormat: 'claude-md',
     emitHooks: 'claude',
     // Project-local Claude Code hooks live in `.claude/settings.json`.

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', () => {
+  test('removes the entire allowed-tools frontmatter block', () => {
-    const frontmatter = [
+    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);
+    const output = rewritePluginMarkdown(input);
-    // The generic path rewrite alone would leave Bash(<skill-base-dir>/scripts/impeccable *),
+    expect(output).not.toMatch(/^allowed-tools:/m);
-    // a dead literal, and any wildcard replacement would auto-approve
-    // same-shaped paths outside the plugin. The line must go entirely.
     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,31 @@ describe('verifyPluginSkillRewrite', () => {
   });
 
   test('fails the build when a launcher pre-approval survives the removal', () => {
-    const reworded = goodSkill.replace(
+    const p = writeSkill(
-      'Bash(.claude/skills/impeccable/scripts/impeccable *)',
+      rewritePluginMarkdown(goodSkill) + '\n  - Bash(<skill-base-dir>/scripts/impeccable.cmd *)\n',
-      'Bash(.claude/skills/impeccable/scripts/impeccable.cmd *)',
     );
-    const p = writeSkill(rewritePluginMarkdown(reworded));
     expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher/);
   });
 
+  test('fails the build when allowed-tools frontmatter survives the removal', () => {
+    const p = writeSkill([
+      '---',
+      'name: impeccable',
+      'allowed-tools:',
+      '  - Bash(npx impeccable *)',
+      'license: Apache 2.0',
+      '---',
+      '',
+      rewritePluginMarkdown(goodSkill),
+    ].join('\n'));
+    expect(() => verifyPluginSkillRewrite(p)).toThrow(/allowed-tools/);
+  });
+
   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
+    // A copy that still carries a node pre-approval must fail the same way as
-    // carries one must fail the same way as a surviving launcher line.
+    // a surviving launcher line, even outside an allowed-tools block.
     const p = writeSkill(
-      rewritePluginMarkdown(goodSkill).replace(
+      rewritePluginMarkdown(goodSkill) + '\n  - Bash(node <skill-base-dir>/scripts/*)\n',
-        'allowed-tools:\n',
-        'allowed-tools:\n  - Bash(node <skill-base-dir>/scripts/*)\n',
-      ),
     );
     expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher or node script path/);
   });
@@ -310,3 +322,14 @@ describe('verifyPluginSkillRewrite', () => {
     expect(() => verifyPluginSkillRewrite(p)).toThrow(/still contains the project-relative scripts path/);
   });
 });
+
+describe('SKILL.src.md frontmatter', () => {
+  test('keeps allowed-tools in source for non-Claude providers (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']).toBeDefined();
+  });
+});