Browse Source

fix: filter available skills by agent config (#227)

Simon Klakegg 4 months ago
parent
commit
6f8b42f0e0

+ 234 - 0
src/hooks/filter-available-skills/index.test.ts

@@ -0,0 +1,234 @@
+import { describe, expect, test } from 'bun:test';
+import type { PluginInput } from '@opencode-ai/plugin';
+import type { PluginConfig } from '../../config';
+import {
+  createFilterAvailableSkillsHook,
+  filterAvailableSkillsText,
+} from './index';
+
+const mockCtx = {} as PluginInput;
+
+function skillBlock(name: string): string {
+  return `<skill>
+  <name>${name}</name>
+  <description>${name} description</description>
+  <location>file:///tmp/${name}</location>
+</skill>`;
+}
+
+function availableSkillsBlock(...names: string[]): string {
+  return `<available_skills>
+${names.map((name) => skillBlock(name)).join('\n')}
+</available_skills>`;
+}
+
+describe('filterAvailableSkillsText', () => {
+  test('keeps only allowed skills using exact skill names', () => {
+    const text = availableSkillsBlock('skill1', 'skill2', 'skill3');
+    const result = filterAvailableSkillsText(text, {
+      '*': 'deny',
+      skill1: 'allow',
+      skill3: 'allow',
+    });
+
+    expect(result).toContain('<name>skill1</name>');
+    expect(result).not.toContain('<name>skill2</name>');
+    expect(result).toContain('<name>skill3</name>');
+  });
+
+  test('renders No skills available when nothing is allowed', () => {
+    const result = filterAvailableSkillsText(availableSkillsBlock('skill1'), {
+      '*': 'deny',
+    });
+
+    expect(result).toContain('No skills available.');
+    expect(result).not.toContain('<name>skill1</name>');
+  });
+});
+
+describe('createFilterAvailableSkillsHook', () => {
+  test('filters system prompt skill blocks for explicit agent skills', async () => {
+    const config: PluginConfig = {
+      agents: {
+        explorer: {
+          skills: ['skill1', 'skill3'],
+        },
+      },
+    };
+
+    const hook = createFilterAvailableSkillsHook(mockCtx, config);
+    const output = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [
+            {
+              type: 'text',
+              text: availableSkillsBlock('skill1', 'skill2', 'skill3'),
+            },
+          ],
+        },
+        {
+          info: { role: 'user', agent: 'explorer' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    const resultText = output.messages[0].parts[0].text;
+    expect(resultText).toContain('<name>skill1</name>');
+    expect(resultText).not.toContain('<name>skill2</name>');
+    expect(resultText).toContain('<name>skill3</name>');
+  });
+
+  test('shows no skills for agents configured with an empty skills list', async () => {
+    const config: PluginConfig = {
+      agents: {
+        fixer: {
+          skills: [],
+        },
+      },
+    };
+
+    const hook = createFilterAvailableSkillsHook(mockCtx, config);
+    const output = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [{ type: 'text', text: availableSkillsBlock('skill1') }],
+        },
+        {
+          info: { role: 'user', agent: 'fixer' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    const resultText = output.messages[0].parts[0].text;
+    expect(resultText).toContain('No skills available.');
+    expect(resultText).not.toContain('<name>skill1</name>');
+  });
+
+  test('preserves orchestrator default wildcard allow', async () => {
+    const hook = createFilterAvailableSkillsHook(mockCtx, {});
+    const output = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [
+            { type: 'text', text: availableSkillsBlock('skill1', 'skill2') },
+          ],
+        },
+        {
+          info: { role: 'user', agent: 'orchestrator' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    const resultText = output.messages[0].parts[0].text;
+    expect(resultText).toContain('<name>skill1</name>');
+    expect(resultText).toContain('<name>skill2</name>');
+  });
+
+  test('supports wildcard allow with explicit exclusions', async () => {
+    const config: PluginConfig = {
+      agents: {
+        designer: {
+          skills: ['*', '!skill2'],
+        },
+      },
+    };
+
+    const hook = createFilterAvailableSkillsHook(mockCtx, config);
+    const output = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [
+            { type: 'text', text: availableSkillsBlock('skill1', 'skill2') },
+          ],
+        },
+        {
+          info: { role: 'user', agent: 'designer' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    const resultText = output.messages[0].parts[0].text;
+    expect(resultText).toContain('<name>skill1</name>');
+    expect(resultText).not.toContain('<name>skill2</name>');
+  });
+
+  test('defaults to orchestrator when no agent is present', async () => {
+    const hook = createFilterAvailableSkillsHook(mockCtx, {});
+    const output = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [{ type: 'text', text: availableSkillsBlock('skill1') }],
+        },
+        {
+          info: { role: 'user' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    expect(output.messages[0].parts[0].text).toContain('<name>skill1</name>');
+  });
+
+  test('filters multiple skill blocks across messages', async () => {
+    const config: PluginConfig = {
+      agents: {
+        explorer: {
+          skills: ['skill1'],
+        },
+      },
+    };
+
+    const hook = createFilterAvailableSkillsHook(mockCtx, config);
+    const output = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [
+            {
+              type: 'text',
+              text: `Intro\n${availableSkillsBlock('skill1', 'skill2')}`,
+            },
+          ],
+        },
+        {
+          info: { role: 'developer' },
+          parts: [
+            { type: 'text', text: availableSkillsBlock('skill2', 'skill3') },
+          ],
+        },
+        {
+          info: { role: 'user', agent: 'explorer' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    expect(output.messages[0].parts[0].text).toContain('<name>skill1</name>');
+    expect(output.messages[0].parts[0].text).not.toContain(
+      '<name>skill2</name>',
+    );
+    expect(output.messages[1].parts[0].text).toContain('No skills available.');
+  });
+});

+ 144 - 0
src/hooks/filter-available-skills/index.ts

@@ -0,0 +1,144 @@
+/**
+ * Filter available_skills block based on the current agent's permission.skill rules.
+ * OpenCode core injects `<available_skills>` globally, so this hook rewrites that
+ * block before the prompt is sent.
+ */
+import type { PluginInput } from '@opencode-ai/plugin';
+import { getSkillPermissionsForAgent } from '../../cli/skills';
+import { getAgentOverride, type PluginConfig } from '../../config';
+
+interface MessageInfo {
+  role: string;
+  agent?: string;
+}
+
+interface MessagePart {
+  type: string;
+  text?: string;
+  [key: string]: unknown;
+}
+
+interface MessageWithParts {
+  info: MessageInfo;
+  parts: MessagePart[];
+}
+
+const AVAILABLE_SKILLS_BLOCK_REGEX =
+  /<available_skills>\s*([\s\S]*?)\s*<\/available_skills>/g;
+const SKILL_NAME_REGEX = /<name>([^<]+)<\/name>/;
+
+type SkillRule = 'allow' | 'ask' | 'deny';
+
+interface SkillEntry {
+  name: string;
+  block: string;
+}
+
+function getCurrentAgent(messages: MessageWithParts[]): string {
+  for (let index = messages.length - 1; index >= 0; index -= 1) {
+    const message = messages[index];
+    if (message.info.role === 'user') {
+      return message.info.agent ?? 'orchestrator';
+    }
+  }
+
+  return 'orchestrator';
+}
+
+function extractSkillEntries(blockContent: string): SkillEntry[] {
+  const entries: SkillEntry[] = [];
+  const skillEntryRegex = /<skill>\s*([\s\S]*?)\s*<\/skill>/g;
+
+  for (const match of blockContent.matchAll(skillEntryRegex)) {
+    const block = match[0];
+    const nameMatch = block.match(SKILL_NAME_REGEX);
+    if (!nameMatch) {
+      continue;
+    }
+
+    entries.push({
+      name: nameMatch[1].trim(),
+      block,
+    });
+  }
+
+  return entries;
+}
+
+function isSkillAllowed(
+  skillName: string,
+  permissionRules: Record<string, SkillRule>,
+): boolean {
+  const specificRule = permissionRules[skillName];
+  if (specificRule !== undefined) {
+    return specificRule === 'allow';
+  }
+
+  return permissionRules['*'] === 'allow';
+}
+
+function filterAvailableSkillsText(
+  text: string,
+  permissionRules: Record<string, SkillRule>,
+): string {
+  return text.replace(
+    AVAILABLE_SKILLS_BLOCK_REGEX,
+    (_fullMatch, blockContent: string) => {
+      const allowedEntries = extractSkillEntries(blockContent).filter((entry) =>
+        isSkillAllowed(entry.name, permissionRules),
+      );
+
+      if (allowedEntries.length === 0) {
+        return '<available_skills>\nNo skills available.\n</available_skills>';
+      }
+
+      return `<available_skills>\n${allowedEntries
+        .map((entry) => entry.block)
+        .join('\n')}\n</available_skills>`;
+    },
+  );
+}
+
+/**
+ * Creates the experimental.chat.messages.transform hook for filtering available skills.
+ * This hook runs right before sending to API, so it doesn't affect UI display.
+ */
+export function createFilterAvailableSkillsHook(
+  _ctx: PluginInput,
+  config: PluginConfig,
+) {
+  return {
+    'experimental.chat.messages.transform': async (
+      _input: Record<string, never>,
+      output: { messages: MessageWithParts[] },
+    ): Promise<void> => {
+      const { messages } = output;
+      if (messages.length === 0) {
+        return;
+      }
+
+      const agentName = getCurrentAgent(messages);
+      const configuredSkills = getAgentOverride(config, agentName)?.skills;
+      const permissionRules = getSkillPermissionsForAgent(
+        agentName,
+        configuredSkills,
+      );
+
+      for (const message of messages) {
+        for (const part of message.parts) {
+          if (
+            part.type !== 'text' ||
+            !part.text ||
+            !part.text.includes('<available_skills>')
+          ) {
+            continue;
+          }
+
+          part.text = filterAvailableSkillsText(part.text, permissionRules);
+        }
+      }
+    },
+  };
+}
+
+export { filterAvailableSkillsText };

+ 1 - 0
src/hooks/index.ts

@@ -2,6 +2,7 @@ export type { AutoUpdateCheckerOptions } from './auto-update-checker';
 export { createAutoUpdateCheckerHook } from './auto-update-checker';
 export { createAutoUpdateCheckerHook } from './auto-update-checker';
 export { createChatHeadersHook } from './chat-headers';
 export { createChatHeadersHook } from './chat-headers';
 export { createDelegateTaskRetryHook } from './delegate-task-retry';
 export { createDelegateTaskRetryHook } from './delegate-task-retry';
+export { createFilterAvailableSkillsHook } from './filter-available-skills';
 export {
 export {
   ForegroundFallbackManager,
   ForegroundFallbackManager,
   isRateLimitError,
   isRateLimitError,

+ 35 - 4
src/index.ts

@@ -8,6 +8,7 @@ import {
   createAutoUpdateCheckerHook,
   createAutoUpdateCheckerHook,
   createChatHeadersHook,
   createChatHeadersHook,
   createDelegateTaskRetryHook,
   createDelegateTaskRetryHook,
+  createFilterAvailableSkillsHook,
   createJsonErrorRecoveryHook,
   createJsonErrorRecoveryHook,
   createPhaseReminderHook,
   createPhaseReminderHook,
   createPostFileToolNudgeHook,
   createPostFileToolNudgeHook,
@@ -124,6 +125,12 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
   // Initialize phase reminder hook for workflow compliance
   // Initialize phase reminder hook for workflow compliance
   const phaseReminderHook = createPhaseReminderHook();
   const phaseReminderHook = createPhaseReminderHook();
 
 
+  // Initialize available skills filter hook
+  const filterAvailableSkillsHook = createFilterAvailableSkillsHook(
+    ctx,
+    config,
+  );
+
   // Initialize post-file-tool nudge hook
   // Initialize post-file-tool nudge hook
   const postFileToolNudgeHook = createPostFileToolNudgeHook();
   const postFileToolNudgeHook = createPostFileToolNudgeHook();
 
 
@@ -290,7 +297,9 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       }
       }
 
 
       // Get all MCP names from the merged config (built-in + custom)
       // Get all MCP names from the merged config (built-in + custom)
-      const mergedMcpConfig = opencodeConfig.mcp as Record<string, unknown> | undefined;
+      const mergedMcpConfig = opencodeConfig.mcp as
+        | Record<string, unknown>
+        | undefined;
       const allMcpNames = Object.keys(mergedMcpConfig ?? mcps);
       const allMcpNames = Object.keys(mergedMcpConfig ?? mcps);
 
 
       // For each agent, create permission rules based on their mcps list
       // For each agent, create permission rules based on their mcps list
@@ -384,9 +393,31 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
 
 
     'chat.headers': chatHeadersHook['chat.headers'],
     'chat.headers': chatHeadersHook['chat.headers'],
 
 
-    // Inject phase reminder before sending to API (doesn't show in UI)
-    'experimental.chat.messages.transform':
-      phaseReminderHook['experimental.chat.messages.transform'],
+    // Inject phase reminder and filter available skills before sending to API (doesn't show in UI)
+    'experimental.chat.messages.transform': async (
+      input: Record<string, never>,
+      output: { messages: unknown[] },
+    ): Promise<void> => {
+      // Type assertion since we know the structure matches MessageWithParts[]
+      const typedOutput = output as {
+        messages: Array<{
+          info: { role: string; agent?: string; sessionID?: string };
+          parts: Array<{
+            type: string;
+            text?: string;
+            [key: string]: unknown;
+          }>;
+        }>;
+      };
+      await phaseReminderHook['experimental.chat.messages.transform'](
+        input,
+        typedOutput,
+      );
+      await filterAvailableSkillsHook['experimental.chat.messages.transform'](
+        input,
+        typedOutput,
+      );
+    },
 
 
     // Post-tool hooks: retry guidance for delegation errors + file-tool nudge
     // Post-tool hooks: retry guidance for delegation errors + file-tool nudge
     'tool.execute.after': async (input, output) => {
     'tool.execute.after': async (input, output) => {