From 456dadca8540322aaa82fdd084ccf41bdecf4958 Mon Sep 17 00:00:00 2001 From: Akhilesh Kumar Date: Thu, 5 Mar 2026 22:32:31 +0000 Subject: [PATCH] feat(core): move subagent policy to declarative TOML and add tool annotations --- packages/core/src/agents/registry.ts | 35 ------------------ packages/core/src/agents/subagent-tool.ts | 1 + .../core/src/policy/policies/subagents.toml | 11 ++++++ .../core/src/policy/policy-engine.test.ts | 3 +- packages/core/src/policy/toml-loader.test.ts | 36 +++++++++++-------- packages/core/src/policy/types.ts | 5 --- 6 files changed, 35 insertions(+), 56 deletions(-) create mode 100644 packages/core/src/policy/policies/subagents.toml diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index bf7e669150..66a2bc3c83 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -23,7 +23,6 @@ import { type ModelConfig, ModelConfigService, } from '../services/modelConfigService.js'; -import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../policy/types.js'; /** * Returns the model config alias for a given agent definition. @@ -315,39 +314,6 @@ export class AgentRegistry { this.agents.set(mergedDefinition.name, mergedDefinition); this.registerModelConfigs(mergedDefinition); - this.addAgentPolicy(mergedDefinition); - } - - private addAgentPolicy(definition: AgentDefinition): void { - const policyEngine = this.config.getPolicyEngine(); - if (!policyEngine) { - return; - } - - // If the user has explicitly defined a policy for this tool, respect it. - // ignoreDynamic=true means we only check for rules NOT added by this registry. - if (policyEngine.hasRuleForTool(definition.name, true)) { - if (this.config.getDebugMode()) { - debugLogger.log( - `[AgentRegistry] User policy exists for '${definition.name}', skipping dynamic registration.`, - ); - } - return; - } - - // Clean up any old dynamic policy for this tool (e.g. if we are overwriting an agent) - policyEngine.removeRulesForTool(definition.name, 'AgentRegistry (Dynamic)'); - - // Add the new dynamic policy - policyEngine.addRule({ - toolName: definition.name, - decision: - definition.kind === 'local' - ? PolicyDecision.ALLOW - : PolicyDecision.ASK_USER, - priority: PRIORITY_SUBAGENT_TOOL, - source: 'AgentRegistry (Dynamic)', - }); } private isAgentEnabled( @@ -461,7 +427,6 @@ export class AgentRegistry { ); } this.agents.set(definition.name, definition); - this.addAgentPolicy(definition); } catch (e) { const errorMessage = `Error loading A2A agent "${definition.name}": ${e instanceof Error ? e.message : String(e)}`; debugLogger.warn(`[AgentRegistry] ${errorMessage}`, e); diff --git a/packages/core/src/agents/subagent-tool.ts b/packages/core/src/agents/subagent-tool.ts index 21a3864160..4a61906080 100644 --- a/packages/core/src/agents/subagent-tool.ts +++ b/packages/core/src/agents/subagent-tool.ts @@ -52,6 +52,7 @@ export class SubagentTool extends BaseDeclarativeTool { messageBus, /* isOutputMarkdown */ true, /* canUpdateOutput */ true, + /* toolAnnotations */ { isSubagent: true, agentKind: definition.kind }, ); } diff --git a/packages/core/src/policy/policies/subagents.toml b/packages/core/src/policy/policies/subagents.toml new file mode 100644 index 0000000000..b1015b77b3 --- /dev/null +++ b/packages/core/src/policy/policies/subagents.toml @@ -0,0 +1,11 @@ +# Default policy for dynamically registered local subagents +[[rule]] +toolAnnotations = { isSubagent = true, agentKind = "local" } +decision = "allow" +priority = 50 + +# Default policy for dynamically registered remote subagents +[[rule]] +toolAnnotations = { isSubagent = true, agentKind = "remote" } +decision = "ask_user" +priority = 50 diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index b8e6968af9..18bc433543 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -13,7 +13,6 @@ import { type SafetyCheckerRule, InProcessCheckerType, ApprovalMode, - PRIORITY_SUBAGENT_TOOL, } from './types.js'; import type { FunctionCall } from '@google/genai'; import { SafetyCheckDecision } from '../safety/protocol.js'; @@ -1595,7 +1594,7 @@ describe('PolicyEngine', () => { { toolName: 'unknown_subagent', decision: PolicyDecision.ALLOW, - priority: PRIORITY_SUBAGENT_TOOL, + priority: 1.05, }, ]; diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 72ffa9ebfb..e8ec77fc31 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -8,7 +8,6 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { PolicyDecision, ApprovalMode, - PRIORITY_SUBAGENT_TOOL, } from './types.js'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; @@ -911,12 +910,27 @@ priority = 100 it('should override default subagent rules when in Plan Mode for unknown subagents', async () => { const planTomlPath = path.resolve(__dirname, 'policies', 'plan.toml'); - const fileContent = await fs.readFile(planTomlPath, 'utf-8'); + const planFileContent = await fs.readFile(planTomlPath, 'utf-8'); + + const subagentsTomlPath = path.resolve( + __dirname, + 'policies', + 'subagents.toml', + ); + const subagentsFileContent = await fs.readFile(subagentsTomlPath, 'utf-8'); + const tempPolicyDir = await fs.mkdtemp( path.join(os.tmpdir(), 'plan-policy-test-'), ); try { - await fs.writeFile(path.join(tempPolicyDir, 'plan.toml'), fileContent); + await fs.writeFile( + path.join(tempPolicyDir, 'plan.toml'), + planFileContent, + ); + await fs.writeFile( + path.join(tempPolicyDir, 'subagents.toml'), + subagentsFileContent, + ); const getPolicyTier = () => 1; // Default tier // 1. Load the actual Plan Mode policies @@ -932,20 +946,14 @@ priority = 100 }); // 3. Simulate an unknown Subagent being registered (Dynamic Rule) - engine.addRule({ - toolName: 'unknown_subagent', - decision: PolicyDecision.ALLOW, - priority: PRIORITY_SUBAGENT_TOOL, - source: 'AgentRegistry (Dynamic)', - }); + // Now handled by subagents.toml policy using toolAnnotations + const checkResult = await engine.check( + { name: 'unknown_subagent' }, + { isSubagent: true, agentKind: 'local' }, + ); // 4. Verify Behavior: // The Plan Mode "Catch-All Deny" (from plan.toml) should override the Subagent Allow - const checkResult = await engine.check( - { name: 'unknown_subagent' }, - undefined, - ); - expect( checkResult.decision, 'Unknown subagent should be DENIED in Plan Mode', diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 18c621c176..e9b09eaff8 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -301,8 +301,3 @@ export interface CheckResult { rule?: PolicyRule; } -/** - * Priority for subagent tools (registered dynamically). - * Effective priority matching Tier 1 (Default) read-only tools. - */ -export const PRIORITY_SUBAGENT_TOOL = 1.05;