Jelajahi Sumber

Merge pull request #431 from alvinunreal/fix/agent-availability-aliases

fix(agents): align council availability and aliases
Alvin 3 bulan lalu
induk
melakukan
db5f81b934

+ 4 - 3
src/agents/councillor.test.ts

@@ -86,16 +86,17 @@ describe('councillor permissions', () => {
     expect(permission.lsp).toBe('allow');
   });
 
-  test('allows list and codesearch tools', () => {
+  test('allows list and code search tools', () => {
     const agent = createCouncillorAgent('test-model');
     const permission = agent.config.permission as Record<string, string>;
     expect(permission.list).toBe('allow');
     expect(permission.codesearch).toBe('allow');
+    expect(permission.ast_grep_search).toBe('allow');
   });
 
-  test('has exactly 8 permission entries', () => {
+  test('has exactly 9 permission entries', () => {
     const agent = createCouncillorAgent('test-model');
     const permission = agent.config.permission as Record<string, string>;
-    expect(Object.keys(permission)).toHaveLength(8);
+    expect(Object.keys(permission)).toHaveLength(9);
   });
 });

+ 1 - 0
src/agents/councillor.ts

@@ -76,6 +76,7 @@ export function createCouncillorAgent(
         lsp: 'allow',
         list: 'allow',
         codesearch: 'allow',
+        ast_grep_search: 'allow',
       },
     },
   };

+ 12 - 0
src/agents/display-name.test.ts

@@ -129,6 +129,18 @@ describe('displayName', () => {
     );
   });
 
+  test('throws when displayName is not a safe agent alias', () => {
+    const config: PluginConfig = {
+      agents: {
+        explorer: { displayName: 'senior reviewer' },
+      },
+    };
+
+    expect(() => createAgents(config)).toThrow(
+      "displayName 'senior reviewer' must match /^[a-z][a-z0-9_-]*$/i",
+    );
+  });
+
   test('resolves legacy alias for explorer displayName override', () => {
     const config: PluginConfig = {
       agents: {

+ 46 - 27
src/agents/index.test.ts

@@ -16,6 +16,13 @@ import {
   isSubagent,
 } from "./index";
 
+function councilConfig() {
+  const parsed = CouncilConfigSchema.parse({
+    presets: { default: { alpha: { model: "test/councillor" } } },
+  });
+  return parsed;
+}
+
 describe("agent alias backward compatibility", () => {
   test("applies 'explore' config to 'explorer' agent", () => {
     const config: PluginConfig = {
@@ -272,7 +279,9 @@ describe("skill permissions", () => {
 
 describe("tool permissions", () => {
   test("council agent is allowed to invoke council_session", () => {
-    const agents = createAgents();
+    const agents = createAgents({
+      council: councilConfig(),
+    });
     const council = agents.find((a) => a.name === "council");
     expect((council?.config.permission as any).council_session).toBe("allow");
   });
@@ -334,7 +343,7 @@ describe("agent classification", () => {
     for (const name of SUBAGENT_NAMES) {
       // Council is a dual-mode agent ("all"), rest are subagents
       if (name === "council") {
-        expect(configs[name].mode).toBe("all");
+        expect(configs[name]).toBeUndefined();
       } else {
         expect(configs[name].mode).toBe("subagent");
       }
@@ -354,9 +363,29 @@ describe("createAgents", () => {
     expect(names).toContain("fixer");
   });
 
-  test("creates exactly 8 agents by default (1 orchestrator + 7 subagents, observer disabled)", () => {
+  test("creates exactly 7 agents by default (observer disabled, council unconfigured)", () => {
     const agents = createAgents();
-    expect(agents.length).toBe(8);
+    expect(agents.length).toBe(7);
+  });
+
+  test("does not create council when council is not configured", () => {
+    const agents = createAgents();
+    const names = agents.map((a) => a.name);
+    const orchestrator = agents.find((a) => a.name === "orchestrator");
+
+    expect(names).not.toContain("council");
+    expect(orchestrator?.config.prompt).not.toContain("@council");
+  });
+
+  test("creates council when council is configured", () => {
+    const agents = createAgents({
+      council: councilConfig(),
+    });
+    const names = agents.map((a) => a.name);
+    const orchestrator = agents.find((a) => a.name === "orchestrator");
+
+    expect(names).toContain("council");
+    expect(orchestrator?.config.prompt).toContain("@council");
   });
 });
 
@@ -379,7 +408,9 @@ describe("getAgentConfigs", () => {
 
 describe("council agent model resolution", () => {
   test("council agent uses default model", () => {
-    const agents = createAgents();
+    const agents = createAgents({
+      council: councilConfig(),
+    });
     const council = agents.find((a) => a.name === "council");
     expect(council?.config.model).toBe(DEFAULT_MODELS.council);
   });
@@ -398,11 +429,7 @@ describe("council agent model resolution", () => {
         oracle: { model: "openai/gpt-5.5" },
       },
       council: {
-        presets: {
-          default: {
-            alpha: { model: "openai/gpt-5.4-mini" },
-          },
-        },
+        ...councilConfig(),
         _legacyMasterModel: "anthropic/claude-opus-4-6",
       },
     };
@@ -418,11 +445,7 @@ describe("council agent model resolution", () => {
         council: { model: "google/gemini-3-pro" },
       },
       council: {
-        presets: {
-          default: {
-            alpha: { model: "openai/gpt-5.4-mini" },
-          },
-        },
+        ...councilConfig(),
         _legacyMasterModel: "anthropic/claude-opus-4-6",
       },
     };
@@ -434,13 +457,7 @@ describe("council agent model resolution", () => {
   test("council uses default when no legacy master and no preset override", () => {
     // No legacy master, no preset override → standard default
     const config: PluginConfig = {
-      council: {
-        presets: {
-          default: {
-            alpha: { model: "openai/gpt-5.4-mini" },
-          },
-        },
-      },
+      council: councilConfig(),
     };
     const agents = createAgents(config);
     const council = agents.find((a) => a.name === "council");
@@ -777,13 +794,13 @@ describe("disabled_agents", () => {
 
   test("agent count decreases when agents are disabled", () => {
     const agents = createAgents();
-    expect(agents.length).toBe(8); // 1 + 7 (observer disabled by default)
+    expect(agents.length).toBe(7); // observer disabled, council unconfigured
 
     const disabledConfig: PluginConfig = {
       disabled_agents: ["observer", "designer"],
     };
     const disabledAgents = createAgents(disabledConfig);
-    expect(disabledAgents.length).toBe(7);
+    expect(disabledAgents.length).toBe(6);
   });
 
   test("getDisabledAgents respects protection rules", () => {
@@ -821,13 +838,15 @@ describe("disabled_agents", () => {
     expect(enabled).not.toContain("janitor");
   });
 
-  test("empty disabled_agents creates all agents including observer", () => {
+  test("empty disabled_agents creates observer but not unconfigured council", () => {
     const config: PluginConfig = {
       disabled_agents: [],
     };
     const agents = createAgents(config);
-    expect(agents.length).toBe(9);
-    expect(agents.map((a) => a.name)).toContain("observer");
+    const names = agents.map((a) => a.name);
+    expect(agents.length).toBe(8);
+    expect(names).toContain("observer");
+    expect(names).not.toContain("council");
   });
 });
 

+ 15 - 0
src/agents/index.ts

@@ -43,6 +43,10 @@ function normalizeDisplayName(displayName: string): string {
   return trimmed.startsWith('@') ? trimmed.slice(1) : trimmed;
 }
 
+function isSafeDisplayName(displayName: string): boolean {
+  return /^[a-z][a-z0-9_-]*$/i.test(displayName);
+}
+
 function escapeRegExp(value: string): string {
   return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
 }
@@ -219,6 +223,9 @@ const SUBAGENT_FACTORIES: Record<SubagentName, AgentFactory> = {
  */
 export function createAgents(config?: PluginConfig): AgentDefinition[] {
   const disabled = getDisabledAgents(config);
+  if (!config?.council) {
+    disabled.add('council');
+  }
 
   // TEMP: If fixer has no config, inherit from librarian's model to avoid breaking
   // existing users who don't have fixer in their config yet
@@ -364,6 +371,11 @@ export function createAgents(config?: PluginConfig): AgentDefinition[] {
   const usedDisplayNames = new Set<string>();
   for (const [, displayName] of displayNameMap) {
     const normalizedDisplayName = normalizeDisplayName(displayName);
+    if (!isSafeDisplayName(normalizedDisplayName)) {
+      throw new Error(
+        `displayName '${normalizedDisplayName}' must match /^[a-z][a-z0-9_-]*$/i`,
+      );
+    }
     if (usedDisplayNames.has(normalizedDisplayName)) {
       throw new Error(
         `Duplicate displayName '${normalizedDisplayName}' assigned to multiple agents`,
@@ -500,6 +512,9 @@ export function getDisabledAgents(config?: PluginConfig): Set<string> {
  */
 export function getEnabledAgentNames(config?: PluginConfig): string[] {
   const disabled = getDisabledAgents(config);
+  if (!config?.council) {
+    disabled.add('council');
+  }
   const customAgentNames = getCustomAgentNames(config).filter(
     (name) => !disabled.has(name),
   );

+ 1 - 1
src/agents/orchestrator.ts

@@ -36,7 +36,7 @@ const AGENT_DESCRIPTIONS: Record<string, string> = {
 
   librarian: `@librarian
 - Role: Authoritative source for current library docs and API references
-- Permissions: None
+- Permissions: External docs/search MCPs; no file edits
 - Stats: 10x better finding up-to-date library docs than orchestrator, 1/2 cost of orchestrator
 - Capabilities: Fetches latest official docs, examples, API signatures, version-specific behavior via grep_app MCP
 - **Delegate when:** Libraries with frequent API changes (React, Next.js, AI SDKs) • Complex APIs needing official examples (ORMs, auth) • Version-specific behavior matters • Unfamiliar library • Edge cases or advanced features • Nuanced best practices