feat: agent delegation rules, skill permission pipeline, and test improvements (#145)

* feat: agent delegation rules, skill permission pipeline, and test improvements

* fix: fixer prompt to delegate file reads to @explorer instead of reading directly

* refactor: make fixer a leaf node with reworked delegation tests

Revert fixer to leaf node (no delegation), rework tests to use designer
as the non-leaf delegator, and remove duplicate chain test covered by
the existing orchestrator → designer → explorer test.

* Modify SUBAGENT_DELEGATION_RULES for designer

Updated delegation rules for 'designer' subagent.

---------

Co-authored-by: Alvin <alvin@cmngoal.com>

.gitignore

@@ -45,6 +45,7 @@ local
 .ignore
 opencode
 oh-my-opencode
+opencode.jsonc
 
 # AI Memory
 .aim/

src/agents/fixer.ts

@@ -14,9 +14,10 @@ const FIXER_PROMPT = `You are Fixer - a fast, focused implementation specialist.
 
 **Constraints**:
 - NO external research (no websearch, context7, grep_app)
-- NO delegation (no background_task)
+- NO delegation (no background_task, no spawning subagents)
 - No multi-step research/planning; minimal execution sequence ok
-- If context is insufficient, read the files listed; only ask for missing inputs you cannot retrieve
+- If context is insufficient: use grep/glob/lsp_diagnostics directly — do not delegate
+- Only ask for missing inputs you truly cannot retrieve yourself
 
 **Output Format**:
 <summary>

src/agents/index.test.ts

@@ -142,6 +142,39 @@ describe('orchestrator agent', () => {
   });
 });
 
+describe('skill permissions', () => {
+  test('orchestrator gets cartography skill allowed by default', () => {
+    const agents = createAgents();
+    const orchestrator = agents.find((a) => a.name === 'orchestrator');
+    expect(orchestrator).toBeDefined();
+    const skillPerm = (
+      orchestrator?.config.permission as Record<string, unknown>
+    )?.skill as Record<string, string>;
+    // orchestrator gets wildcard allow (from RECOMMENDED_SKILLS wildcard entry)
+    expect(skillPerm?.['*']).toBe('allow');
+    // CUSTOM_SKILLS loop must also add a named cartography entry for orchestrator
+    expect(skillPerm?.cartography).toBe('allow');
+  });
+
+  test('explorer gets cartography skill allowed by default', () => {
+    const agents = createAgents();
+    const explorer = agents.find((a) => a.name === 'explorer');
+    expect(explorer).toBeDefined();
+    const skillPerm = (explorer?.config.permission as Record<string, unknown>)
+      ?.skill as Record<string, string>;
+    expect(skillPerm?.cartography).toBe('allow');
+  });
+
+  test('oracle gets requesting-code-review skill allowed by default', () => {
+    const agents = createAgents();
+    const oracle = agents.find((a) => a.name === 'oracle');
+    expect(oracle).toBeDefined();
+    const skillPerm = (oracle?.config.permission as Record<string, unknown>)
+      ?.skill as Record<string, string>;
+    expect(skillPerm?.['requesting-code-review']).toBe('allow');
+  });
+});
+
 describe('isSubagent type guard', () => {
   test('returns true for valid subagent names', () => {
     expect(isSubagent('explorer')).toBe(true);

src/agents/orchestrator.ts

@@ -102,6 +102,10 @@ Balance: respect dependencies, avoid parallelizing what must be sequential.
 - Confirm specialists completed successfully
 - Verify solution meets requirements
 
+## Agent Role Mapping
+When a workflow calls for an **implementer** subagent: dispatch \`@fixer\`. Fixer has enforced constraints (no research, no delegation, structured output) that match the implementer role exactly.
+When a workflow calls for a **reviewer** subagent: dispatch \`@oracle\`. Oracle has the depth for architectural review and access to code review skills.
+
 </Workflow>
 
 <Communication>

src/background/background-manager.test.ts

@@ -748,13 +748,13 @@ describe('BackgroundTaskManager', () => {
       });
     });
 
-    test('spawned fixer gets tools enabled (can delegate to explorer)', async () => {
+    test('spawned designer gets tools enabled (can delegate to explorer)', async () => {
       const ctx = createMockContext();
       const manager = new BackgroundTaskManager(ctx);
 
-      // First, launch an explorer task
-      const explorerTask = manager.launch({
-        agent: 'explorer',
+      // First, launch an orchestrator task
+      const orchestratorTask = manager.launch({
+        agent: 'orchestrator',
         prompt: 'test',
         description: 'test',
         parentSessionId: 'root-session',
@@ -763,22 +763,22 @@ describe('BackgroundTaskManager', () => {
       await Promise.resolve();
       await Promise.resolve();
 
-      // Launch fixer from explorer - fixer can delegate to explorer, so tools enabled
-      const explorerSessionId = explorerTask.sessionId;
-      if (!explorerSessionId)
+      // Launch designer from orchestrator - designer can delegate to explorer, so tools enabled
+      const orchestratorSessionId = orchestratorTask.sessionId;
+      if (!orchestratorSessionId)
         throw new Error('Expected sessionId to be defined');
 
       manager.launch({
-        agent: 'fixer',
+        agent: 'designer',
         prompt: 'test',
         description: 'test',
-        parentSessionId: explorerSessionId,
+        parentSessionId: orchestratorSessionId,
       });
 
       await Promise.resolve();
       await Promise.resolve();
 
-      // Fixer can delegate (to explorer), so delegation tools are enabled
+      // Designer can delegate (to explorer), so delegation tools are enabled
       const promptCalls = ctx.client.session.prompt.mock.calls as Array<
         [{ body: { tools?: Record<string, boolean> } }]
       >;
@@ -789,45 +789,6 @@ describe('BackgroundTaskManager', () => {
       });
     });
 
-    test('spawned explorer from fixer gets tools disabled (leaf node)', async () => {
-      const ctx = createMockContext();
-      const manager = new BackgroundTaskManager(ctx);
-
-      // Launch a fixer task
-      const fixerTask = manager.launch({
-        agent: 'fixer',
-        prompt: 'test',
-        description: 'test',
-        parentSessionId: 'root-session',
-      });
-
-      await Promise.resolve();
-      await Promise.resolve();
-
-      // Launch explorer from fixer - explorer is a leaf node so tools disabled
-      const fixerSessionId = fixerTask.sessionId;
-      if (!fixerSessionId) throw new Error('Expected sessionId to be defined');
-
-      manager.launch({
-        agent: 'explorer',
-        prompt: 'test',
-        description: 'test',
-        parentSessionId: fixerSessionId,
-      });
-
-      await Promise.resolve();
-      await Promise.resolve();
-
-      const promptCalls = ctx.client.session.prompt.mock.calls as Array<
-        [{ body: { tools?: Record<string, boolean> } }]
-      >;
-      const lastCall = promptCalls[promptCalls.length - 1];
-      expect(lastCall[0].body.tools).toEqual({
-        background_task: false,
-        task: false,
-      });
-    });
-
     test('spawned explorer from designer gets tools disabled (leaf node)', async () => {
       const ctx = createMockContext();
       const manager = new BackgroundTaskManager(ctx);
@@ -1024,11 +985,10 @@ describe('BackgroundTaskManager', () => {
       const fixerSessionId = fixerTask.sessionId;
       if (!fixerSessionId) throw new Error('Expected sessionId to be defined');
 
-      // Fixer can only delegate to explorer
-      expect(manager.isAgentAllowed(fixerSessionId, 'explorer')).toBe(true);
+      // Fixer cannot delegate to any subagents
+      expect(manager.isAgentAllowed(fixerSessionId, 'explorer')).toBe(false);
       expect(manager.isAgentAllowed(fixerSessionId, 'oracle')).toBe(false);
       expect(manager.isAgentAllowed(fixerSessionId, 'designer')).toBe(false);
-      expect(manager.isAgentAllowed(fixerSessionId, 'librarian')).toBe(false);
     });
 
     test('isAgentAllowed returns false for leaf agents', async () => {
@@ -1151,85 +1111,6 @@ describe('BackgroundTaskManager', () => {
       });
     });
 
-    test('full chain: orchestrator → fixer → explorer', async () => {
-      const ctx = createMockContext();
-      const manager = new BackgroundTaskManager(ctx);
-
-      // Level 1: Launch orchestrator from root
-      const orchestratorTask = manager.launch({
-        agent: 'orchestrator',
-        prompt: 'coordinate work',
-        description: 'orchestrator',
-        parentSessionId: 'root-session',
-      });
-
-      await Promise.resolve();
-      await Promise.resolve();
-
-      const orchestratorSessionId = orchestratorTask.sessionId;
-      if (!orchestratorSessionId)
-        throw new Error('Expected sessionId to be defined');
-
-      // Orchestrator can delegate to fixer
-      expect(manager.isAgentAllowed(orchestratorSessionId, 'fixer')).toBe(true);
-
-      // Level 2: Launch fixer from orchestrator
-      const fixerTask = manager.launch({
-        agent: 'fixer',
-        prompt: 'implement changes',
-        description: 'fixer',
-        parentSessionId: orchestratorSessionId,
-      });
-
-      await Promise.resolve();
-      await Promise.resolve();
-
-      const fixerSessionId = fixerTask.sessionId;
-      if (!fixerSessionId) throw new Error('Expected sessionId to be defined');
-
-      // Fixer gets tools ENABLED (can delegate to explorer)
-      const promptCalls = ctx.client.session.prompt.mock.calls as Array<
-        [{ body: { tools?: Record<string, boolean> } }]
-      >;
-      const fixerPromptCall = promptCalls[1]; // Second prompt call is fixer
-      expect(fixerPromptCall[0].body.tools).toEqual({
-        background_task: true,
-        task: true,
-      });
-
-      // Fixer can delegate to explorer but NOT oracle
-      expect(manager.isAgentAllowed(fixerSessionId, 'explorer')).toBe(true);
-      expect(manager.isAgentAllowed(fixerSessionId, 'oracle')).toBe(false);
-
-      // Level 3: Launch explorer from fixer
-      const explorerTask = manager.launch({
-        agent: 'explorer',
-        prompt: 'search codebase',
-        description: 'explorer',
-        parentSessionId: fixerSessionId,
-      });
-
-      await Promise.resolve();
-      await Promise.resolve();
-
-      const explorerSessionId = explorerTask.sessionId;
-      if (!explorerSessionId)
-        throw new Error('Expected sessionId to be defined');
-
-      // Explorer gets tools DISABLED (leaf node)
-      const explorerPromptCall = promptCalls[2]; // Third prompt call is explorer
-      expect(explorerPromptCall[0].body.tools).toEqual({
-        background_task: false,
-        task: false,
-      });
-
-      // Explorer cannot delegate to anything
-      expect(manager.isAgentAllowed(explorerSessionId, 'explorer')).toBe(false);
-      expect(manager.isAgentAllowed(explorerSessionId, 'fixer')).toBe(false);
-      expect(manager.isAgentAllowed(explorerSessionId, 'oracle')).toBe(false);
-      expect(manager.getAllowedSubagents(explorerSessionId)).toEqual([]);
-    });
-
     test('full chain: orchestrator → designer → explorer', async () => {
       const ctx = createMockContext();
       const manager = new BackgroundTaskManager(ctx);
@@ -1343,9 +1224,9 @@ describe('BackgroundTaskManager', () => {
       expect(manager.isAgentAllowed(fixerSessionId, 'librarian')).toBe(false);
       expect(manager.isAgentAllowed(fixerSessionId, 'fixer')).toBe(false);
 
-      // Only explorer is allowed
-      expect(manager.isAgentAllowed(fixerSessionId, 'explorer')).toBe(true);
-      expect(manager.getAllowedSubagents(fixerSessionId)).toEqual(['explorer']);
+      // Explorer is also blocked (fixer is a leaf node)
+      expect(manager.isAgentAllowed(fixerSessionId, 'explorer')).toBe(false);
+      expect(manager.getAllowedSubagents(fixerSessionId)).toEqual([]);
     });
 
     test('chain: completed parent does not affect child permissions', async () => {
@@ -1361,9 +1242,9 @@ describe('BackgroundTaskManager', () => {
       });
       const manager = new BackgroundTaskManager(ctx);
 
-      // Launch fixer
-      const fixerTask = manager.launch({
-        agent: 'fixer',
+      // Launch designer
+      const designerTask = manager.launch({
+        agent: 'designer',
         prompt: 'test',
         description: 'test',
         parentSessionId: 'root-session',
@@ -1372,15 +1253,16 @@ describe('BackgroundTaskManager', () => {
       await Promise.resolve();
       await Promise.resolve();
 
-      const fixerSessionId = fixerTask.sessionId;
-      if (!fixerSessionId) throw new Error('Expected sessionId to be defined');
+      const designerSessionId = designerTask.sessionId;
+      if (!designerSessionId)
+        throw new Error('Expected sessionId to be defined');
 
-      // Launch explorer from fixer BEFORE fixer completes
+      // Launch explorer from designer BEFORE designer completes
       const explorerTask = manager.launch({
         agent: 'explorer',
         prompt: 'test',
         description: 'test',
-        parentSessionId: fixerSessionId,
+        parentSessionId: designerSessionId,
       });
 
       await Promise.resolve();
@@ -1400,16 +1282,16 @@ describe('BackgroundTaskManager', () => {
         task: false,
       });
 
-      // Now complete the fixer (cleans up fixer's agentBySessionId entry)
+      // Now complete the designer (cleans up designer's agentBySessionId entry)
       await manager.handleSessionStatus({
         type: 'session.status',
         properties: {
-          sessionID: fixerSessionId,
+          sessionID: designerSessionId,
           status: { type: 'idle' },
         },
       });
 
-      expect(fixerTask.status).toBe('completed');
+      expect(designerTask.status).toBe('completed');
 
       // Explorer's own session tracking is independent — still works
       expect(manager.isAgentAllowed(explorerSessionId, 'fixer')).toBe(false);
@@ -1443,7 +1325,7 @@ describe('BackgroundTaskManager', () => {
         'fixer',
       ]);
 
-      // Fixer -> only explorer
+      // Fixer -> empty (leaf node)
       const fixerTask = manager.launch({
         agent: 'fixer',
         prompt: 'test',
@@ -1457,7 +1339,7 @@ describe('BackgroundTaskManager', () => {
       const fixerSessionId = fixerTask.sessionId;
       if (!fixerSessionId) throw new Error('Expected sessionId to be defined');
 
-      expect(manager.getAllowedSubagents(fixerSessionId)).toEqual(['explorer']);
+      expect(manager.getAllowedSubagents(fixerSessionId)).toEqual([]);
 
       // Designer -> only explorer
       const designerTask = manager.launch({

src/cli/custom-skills.ts

@@ -31,7 +31,7 @@ export const CUSTOM_SKILLS: CustomSkill[] = [
   {
     name: 'cartography',
     description: 'Repository understanding and hierarchical codemap generation',
-    allowedAgents: ['orchestrator'],
+    allowedAgents: ['orchestrator', 'explorer'],
     sourcePath: 'src/skills/cartography',
   },
 ];

src/cli/skills.ts

@@ -1,4 +1,5 @@
 import { spawnSync } from 'node:child_process';
+import { CUSTOM_SKILLS } from './custom-skills';
 
 /**
  * A recommended skill to install via `npx skills add`.
@@ -18,6 +19,19 @@ export interface RecommendedSkill {
   postInstallCommands?: string[];
 }
 
+/**
+ * A skill that is managed externally (e.g. user-installed) and needs
+ * permission grants but is NOT installed by this plugin's CLI.
+ */
+export interface PermissionOnlySkill {
+  /** Skill name — must match the name OpenCode uses for permission checks */
+  name: string;
+  /** List of agents that should auto-allow this skill */
+  allowedAgents: string[];
+  /** Human-readable description (for documentation only) */
+  description: string;
+}
+
 /**
  * List of recommended skills.
  * Add new skills here to include them in the installation flow.
@@ -43,6 +57,19 @@ export const RECOMMENDED_SKILLS: RecommendedSkill[] = [
   },
 ];
 
+/**
+ * Skills managed externally (not installed by this plugin's CLI).
+ * Entries here only affect agent permission grants — nothing is installed.
+ */
+export const PERMISSION_ONLY_SKILLS: PermissionOnlySkill[] = [
+  {
+    name: 'requesting-code-review',
+    allowedAgents: ['oracle'],
+    description:
+      'Code review template for reviewer subagents in multi-step workflows',
+  },
+];
+
 /**
  * Install a skill using `npx skills add`.
  * @param skill - The skill to install
@@ -127,5 +154,25 @@ export function getSkillPermissionsForAgent(
     }
   }
 
+  // Apply permissions from bundled custom skills
+  for (const skill of CUSTOM_SKILLS) {
+    const isAllowed =
+      skill.allowedAgents.includes('*') ||
+      skill.allowedAgents.includes(agentName);
+    if (isAllowed) {
+      permissions[skill.name] = 'allow';
+    }
+  }
+
+  // Apply permissions for externally-managed skills (not installed by this plugin)
+  for (const skill of PERMISSION_ONLY_SKILLS) {
+    const isAllowed =
+      skill.allowedAgents.includes('*') ||
+      skill.allowedAgents.includes(agentName);
+    if (isAllowed) {
+      permissions[skill.name] = 'allow';
+    }
+  }
+
   return permissions;
 }

src/config/constants.ts

@@ -21,14 +21,14 @@ export type AgentName = (typeof ALL_AGENT_NAMES)[number];
 
 // Subagent delegation rules: which agents can spawn which subagents
 // orchestrator: can spawn all subagents (full delegation)
-// fixer: can spawn explorer (for research during implementation)
+// fixer: leaf node — prompt forbids delegation; use grep/glob for lookups
 // designer: can spawn explorer (for research during design)
 // explorer/librarian/oracle: cannot spawn any subagents (leaf nodes)
 // Unknown agent types not listed here default to explorer-only access
 export const SUBAGENT_DELEGATION_RULES: Record<AgentName, readonly string[]> = {
   orchestrator: SUBAGENT_NAMES,
-  fixer: ['explorer'],
-  designer: ['explorer'],
+  fixer: [],
+  designer: [],
   explorer: [],
   librarian: [],
   oracle: [],