feat(core): honor skill allowedTools by auto-approving declared tools

packages/cli/src/services/BundledSkillLoader.test.ts

@@ -33,16 +33,21 @@ describe('BundledSkillLoader', () => {
   let mockSkillManager: {
     listSkills: ReturnType<typeof vi.fn>;
   };
+  let mockAddSessionAllowRule: ReturnType<typeof vi.fn>;
 
   beforeEach(() => {
     vi.clearAllMocks();
     mockSkillManager = {
       listSkills: vi.fn().mockResolvedValue([]),
     };
+    mockAddSessionAllowRule = vi.fn();
     mockConfig = {
       getSkillManager: vi.fn().mockReturnValue(mockSkillManager),
       isCronEnabled: vi.fn().mockReturnValue(false),
       getModel: vi.fn().mockReturnValue(undefined),
+      getPermissionManager: vi
+        .fn()
+        .mockReturnValue({ addSessionAllowRule: mockAddSessionAllowRule }),
     } as unknown as Config;
   });
 
@@ -155,6 +160,38 @@ describe('BundledSkillLoader', () => {
     });
   });
 
+  describe('allowedTools grant', () => {
+    it('grants allowedTools as session allow rules when the command runs', async () => {
+      const skill = makeSkill({ allowedTools: ['Bash(git *)', 'Edit'] });
+      mockSkillManager.listSkills.mockResolvedValue([skill]);
+
+      const loader = new BundledSkillLoader(mockConfig);
+      const commands = await loader.loadCommands(signal);
+      await commands[0].action!(
+        { invocation: { raw: '/review', args: '' } } as never,
+        '',
+      );
+
+      expect(mockAddSessionAllowRule).toHaveBeenCalledTimes(2);
+      expect(mockAddSessionAllowRule).toHaveBeenNthCalledWith(1, 'Bash(git *)');
+      expect(mockAddSessionAllowRule).toHaveBeenNthCalledWith(2, 'Edit');
+    });
+
+    it('does not grant when the bundled skill declares no allowedTools', async () => {
+      const skill = makeSkill(); // no allowedTools
+      mockSkillManager.listSkills.mockResolvedValue([skill]);
+
+      const loader = new BundledSkillLoader(mockConfig);
+      const commands = await loader.loadCommands(signal);
+      await commands[0].action!(
+        { invocation: { raw: '/review', args: '' } } as never,
+        '',
+      );
+
+      expect(mockAddSessionAllowRule).not.toHaveBeenCalled();
+    });
+  });
+
   it('should return empty array when listSkills throws', async () => {
     mockSkillManager.listSkills.mockRejectedValue(new Error('load failed'));
 

packages/cli/src/services/BundledSkillLoader.ts

@@ -9,6 +9,7 @@ import {
   createDebugLogger,
   appendToLastTextPart,
   buildSkillLlmContent,
+  applySkillAllowedTools,
 } from '@qwen-code/qwen-code-core';
 import { dirname } from 'node:path';
 import type { ICommandLoader } from './types.js';
@@ -73,6 +74,12 @@ export class BundledSkillLoader implements ICommandLoader {
         argumentHint: skill.argumentHint,
         whenToUse: skill.whenToUse,
         action: async (context, _args): Promise<SlashCommandActionReturn> => {
+          // Auto-approve the skill's declared allowedTools before its body is submitted.
+          applySkillAllowedTools(
+            this.config?.getPermissionManager(),
+            skill.allowedTools,
+          );
+
           // Resolve template variables in skill body
           let body = skill.body;
           const modelId = this.config?.getModel()?.trim() || '';

packages/cli/src/services/SkillCommandLoader.test.ts

@@ -6,7 +6,7 @@
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { SkillCommandLoader } from './SkillCommandLoader.js';
-import { CommandKind } from '../ui/commands/types.js';
+import { CommandKind, type CommandContext } from '../ui/commands/types.js';
 import {
   buildSkillLlmContent,
   type Config,
@@ -31,15 +31,20 @@ function makeSkillPrompt(body: string): string {
 describe('SkillCommandLoader', () => {
   let mockConfig: Config;
   let mockSkillManager: { listSkills: ReturnType<typeof vi.fn> };
+  let mockAddSessionAllowRule: ReturnType<typeof vi.fn>;
 
   beforeEach(() => {
     vi.clearAllMocks();
     mockSkillManager = {
       listSkills: vi.fn().mockResolvedValue([]),
     };
+    mockAddSessionAllowRule = vi.fn();
     mockConfig = {
       getSkillManager: vi.fn().mockReturnValue(mockSkillManager),
       getBareMode: vi.fn().mockReturnValue(false),
+      getPermissionManager: vi
+        .fn()
+        .mockReturnValue({ addSessionAllowRule: mockAddSessionAllowRule }),
     } as unknown as Config;
   });
 
@@ -331,4 +336,39 @@ describe('SkillCommandLoader', () => {
       'ext-skill',
     ]);
   });
+
+  describe('allowedTools grant', () => {
+    it('grants allowedTools as session allow rules when the command runs', async () => {
+      const skill = makeSkill({
+        level: 'user',
+        allowedTools: ['Bash(git *)', 'Edit'],
+      });
+      mockSkillManager.listSkills.mockImplementation(
+        ({ level }: { level: string }) =>
+          Promise.resolve(level === 'user' ? [skill] : []),
+      );
+
+      const loader = new SkillCommandLoader(mockConfig);
+      const commands = await loader.loadCommands(signal);
+      await commands[0].action?.({} as CommandContext, '');
+
+      expect(mockAddSessionAllowRule).toHaveBeenCalledTimes(2);
+      expect(mockAddSessionAllowRule).toHaveBeenNthCalledWith(1, 'Bash(git *)');
+      expect(mockAddSessionAllowRule).toHaveBeenNthCalledWith(2, 'Edit');
+    });
+
+    it('does not grant when the skill declares no allowedTools', async () => {
+      const skill = makeSkill({ level: 'user' }); // no allowedTools
+      mockSkillManager.listSkills.mockImplementation(
+        ({ level }: { level: string }) =>
+          Promise.resolve(level === 'user' ? [skill] : []),
+      );
+
+      const loader = new SkillCommandLoader(mockConfig);
+      const commands = await loader.loadCommands(signal);
+      await commands[0].action?.({} as CommandContext, '');
+
+      expect(mockAddSessionAllowRule).not.toHaveBeenCalled();
+    });
+  });
 });

packages/cli/src/services/SkillCommandLoader.ts

@@ -9,6 +9,7 @@ import {
   createDebugLogger,
   appendToLastTextPart,
   buildSkillLlmContent,
+  applySkillAllowedTools,
 } from '@qwen-code/qwen-code-core';
 import { dirname } from 'node:path';
 import type { ICommandLoader } from './types.js';
@@ -96,6 +97,12 @@ export class SkillCommandLoader implements ICommandLoader {
           argumentHint: skill.argumentHint,
           whenToUse: skill.whenToUse,
           action: async (context, _args): Promise<SlashCommandActionReturn> => {
+            // Auto-approve the skill's declared allowedTools before its body is submitted.
+            applySkillAllowedTools(
+              this.config?.getPermissionManager(),
+              skill.allowedTools,
+            );
+
             const body = buildSkillLlmContent(
               dirname(skill.filePath),
               skill.body,

packages/core/src/index.ts

@@ -88,7 +88,10 @@ export * from './tools/sdk-control-client-transport.js';
 export * from './tools/modifiable-tool.js';
 
 // Selective re-exports of types/utilities from tool files (avoids loading full tool modules)
-export { buildSkillLlmContent } from './tools/skill-utils.js';
+export {
+  buildSkillLlmContent,
+  applySkillAllowedTools,
+} from './tools/skill-utils.js';
 
 // Backward-compatible type re-exports for tool classes removed from eager loading.
 // These preserve TypeScript type compatibility for downstream consumers.

packages/core/src/skills/types.ts

@@ -36,8 +36,16 @@ export interface SkillConfig {
   description: string;
 
   /**
-   * Optional list of tool names that this skill is allowed to use.
-   * For v1, this is informational only (no gating).
+   * Optional list of tools to auto-approve while this skill is active.
+   *
+   * Each entry is a permission rule string in the same syntax as
+   * `settings.json` `permissions.allow` — e.g. `Bash(git *)`, `Edit`, `Read`,
+   * `mcp__server__tool`. When the skill is invoked (via the model Skill-tool or
+   * a user `/<skill>` slash command), each entry is added as a session-scoped
+   * allow rule, so matching tool calls are auto-approved instead of prompting.
+   *
+   * This is an additive grant only: it never hides or restricts the tools the
+   * model can see. Malformed entries are ignored. See `applySkillAllowedTools`.
    */
   allowedTools?: string[];
 

packages/core/src/tools/skill-utils.test.ts

@@ -0,0 +1,63 @@
+/**
+ * @license
+ * Copyright 2025 Qwen
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { applySkillAllowedTools } from './skill-utils.js';
+import type { PermissionManager } from '../permissions/permission-manager.js';
+
+function mockPermissionManager(): {
+  pm: PermissionManager;
+  addSessionAllowRule: ReturnType<typeof vi.fn>;
+} {
+  const addSessionAllowRule = vi.fn();
+  return {
+    pm: { addSessionAllowRule } as unknown as PermissionManager,
+    addSessionAllowRule,
+  };
+}
+
+describe('applySkillAllowedTools', () => {
+  it('adds one session allow rule per entry, verbatim and in order', () => {
+    const { pm, addSessionAllowRule } = mockPermissionManager();
+
+    applySkillAllowedTools(pm, ['Bash(git *)', 'Edit', 'mcp__server__tool']);
+
+    expect(addSessionAllowRule).toHaveBeenCalledTimes(3);
+    expect(addSessionAllowRule).toHaveBeenNthCalledWith(1, 'Bash(git *)');
+    expect(addSessionAllowRule).toHaveBeenNthCalledWith(2, 'Edit');
+    expect(addSessionAllowRule).toHaveBeenNthCalledWith(3, 'mcp__server__tool');
+  });
+
+  it('no-ops when allowedTools is undefined', () => {
+    const { pm, addSessionAllowRule } = mockPermissionManager();
+    applySkillAllowedTools(pm, undefined);
+    expect(addSessionAllowRule).not.toHaveBeenCalled();
+  });
+
+  it('no-ops when allowedTools is empty', () => {
+    const { pm, addSessionAllowRule } = mockPermissionManager();
+    applySkillAllowedTools(pm, []);
+    expect(addSessionAllowRule).not.toHaveBeenCalled();
+  });
+
+  it('no-ops without throwing when there is no permission manager', () => {
+    expect(() => applySkillAllowedTools(null, ['Bash(git *)'])).not.toThrow();
+    expect(() =>
+      applySkillAllowedTools(undefined, ['Bash(git *)']),
+    ).not.toThrow();
+  });
+
+  it('delegates malformed-entry handling to the permission manager (does not pre-filter)', () => {
+    // The permission manager is the single authority on rule validity; the
+    // helper forwards every entry and lets addSessionAllowRule log/skip bad
+    // ones. This keeps validation in one place.
+    const { pm, addSessionAllowRule } = mockPermissionManager();
+    applySkillAllowedTools(pm, ['Bash(unbalanced', 'Read']);
+    expect(addSessionAllowRule).toHaveBeenCalledTimes(2);
+    expect(addSessionAllowRule).toHaveBeenNthCalledWith(1, 'Bash(unbalanced');
+    expect(addSessionAllowRule).toHaveBeenNthCalledWith(2, 'Read');
+  });
+});

packages/core/src/tools/skill-utils.ts

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import type { PermissionManager } from '../permissions/permission-manager.js';
+
 /**
  * Builds the LLM-facing content string when a skill body is injected.
  * Shared between SkillToolInvocation (runtime) and /context (estimation)
@@ -12,3 +14,26 @@
 export function buildSkillLlmContent(baseDir: string, body: string): string {
   return `Base directory for this skill: ${baseDir}\nImportant: ALWAYS resolve absolute paths from this base directory when working with skills.\n\n${body}\n`;
 }
+
+/**
+ * Grants a skill's `allowedTools` as session-scoped permission allow rules.
+ *
+ * Each entry is a permission rule string in the same syntax as `settings.json`
+ * `permissions.allow` (e.g. `Bash(git *)`, `Edit`, `mcp__server__tool`) and is
+ * handed verbatim to the session allow list, so matching tool calls are
+ * auto-approved for the rest of the session instead of prompting. This is an
+ * additive grant only — it never hides or restricts the tools the model sees.
+ *
+ * No-ops when there is no permission manager or nothing to grant.
+ */
+export function applySkillAllowedTools(
+  permissionManager: PermissionManager | null | undefined,
+  allowedTools: string[] | undefined,
+): void {
+  if (!permissionManager || !allowedTools?.length) {
+    return;
+  }
+  for (const rule of allowedTools) {
+    permissionManager.addSessionAllowRule(rule);
+  }
+}

packages/core/src/tools/skill.test.ts

@@ -49,6 +49,7 @@ describe('SkillTool', () => {
   let skillTool: SkillTool;
   let mockSkillManager: SkillManager;
   let changeListeners: Array<() => void>;
+  let mockAddSessionAllowRule: ReturnType<typeof vi.fn>;
 
   const mockSkills: SkillConfig[] = [
     {
@@ -72,6 +73,8 @@ describe('SkillTool', () => {
     // Setup fake timers
     vi.useFakeTimers();
 
+    mockAddSessionAllowRule = vi.fn();
+
     // Create mock config
     config = {
       getProjectRoot: vi.fn().mockReturnValue('/test/project'),
@@ -80,6 +83,9 @@ describe('SkillTool', () => {
       getGeminiClient: vi.fn().mockReturnValue(undefined),
       getModelInvocableCommandsProvider: vi.fn().mockReturnValue(null),
       getModelInvocableCommandsExecutor: vi.fn().mockReturnValue(null),
+      getPermissionManager: vi
+        .fn()
+        .mockReturnValue({ addSessionAllowRule: mockAddSessionAllowRule }),
     } as unknown as Config;
 
     changeListeners = [];
@@ -521,6 +527,36 @@ describe('SkillTool', () => {
       expect(result.returnDisplay).toBe('Skill for writing and running tests');
     });
 
+    it('grants allowedTools as session allow rules on invocation', async () => {
+      vi.mocked(mockSkillManager.loadSkillForRuntime).mockResolvedValue({
+        ...mockSkills[1],
+        allowedTools: ['Bash(git *)', 'Edit'],
+      });
+
+      const invocation = (
+        skillTool as SkillToolWithProtectedMethods
+      ).createInvocation({ skill: 'testing' });
+      await invocation.execute();
+
+      expect(mockAddSessionAllowRule).toHaveBeenCalledTimes(2);
+      expect(mockAddSessionAllowRule).toHaveBeenNthCalledWith(1, 'Bash(git *)');
+      expect(mockAddSessionAllowRule).toHaveBeenNthCalledWith(2, 'Edit');
+    });
+
+    it('does not add allow rules when the skill declares no allowedTools', async () => {
+      // code-review (mockSkills[0]) has no allowedTools field.
+      vi.mocked(mockSkillManager.loadSkillForRuntime).mockResolvedValue(
+        mockSkills[0],
+      );
+
+      const invocation = (
+        skillTool as SkillToolWithProtectedMethods
+      ).createInvocation({ skill: 'code-review' });
+      await invocation.execute();
+
+      expect(mockAddSessionAllowRule).not.toHaveBeenCalled();
+    });
+
     it('should handle skill not found error', async () => {
       vi.mocked(mockSkillManager.loadSkillForRuntime).mockResolvedValue(null);
 

packages/core/src/tools/skill.ts

@@ -25,7 +25,7 @@ export interface SkillParams {
 
 // Re-export for backward compatibility
 export { buildSkillLlmContent } from './skill-utils.js';
-import { buildSkillLlmContent } from './skill-utils.js';
+import { buildSkillLlmContent, applySkillAllowedTools } from './skill-utils.js';
 
 /**
  * Skill tool that enables the model to access skill definitions.
@@ -437,6 +437,12 @@ class SkillToolInvocation extends BaseToolInvocation<SkillParams, ToolResult> {
       );
       this.onSkillLoaded(this.params.skill);
 
+      // Auto-approve the skill's declared allowedTools for the rest of the session.
+      applySkillAllowedTools(
+        this.config.getPermissionManager(),
+        skill.allowedTools,
+      );
+
       // Register skill hooks if present
       debugLogger.debug('Skill hooks check:', {
         hasHooks: !!skill.hooks,