Implementation for the policy changes for subagents

This commit is contained in:
Akhilesh Kumar
2026-03-02 21:33:18 +00:00
parent 7c9fceba7f
commit 03c6714c6d
10 changed files with 434 additions and 40 deletions
+12
View File
@@ -16,6 +16,12 @@ import {
DEFAULT_MAX_TIME_MINUTES,
} from './types.js';
import type { A2AAuthConfig } from './auth-provider/types.js';
import { type MCPServerConfig } from '../config/config.js';
import { type PolicySettings } from '../policy/types.js';
import {
PolicySettingsSchema,
MCPServersConfigSchema,
} from '../policy/schemas.js';
import { isValidToolName } from '../tools/tool-names.js';
import { FRONTMATTER_REGEX } from '../skills/skillLoader.js';
import { getErrorMessage } from '../utils/errors.js';
@@ -38,6 +44,8 @@ interface FrontmatterLocalAgentDefinition
temperature?: number;
max_turns?: number;
timeout_mins?: number;
policy?: PolicySettings;
mcp_servers?: Record<string, MCPServerConfig>;
}
/**
@@ -110,6 +118,8 @@ const localAgentSchema = z
temperature: z.number().optional(),
max_turns: z.number().int().positive().optional(),
timeout_mins: z.number().int().positive().optional(),
policy: PolicySettingsSchema.optional(),
mcp_servers: MCPServersConfigSchema.optional(),
})
.strict();
@@ -456,6 +466,8 @@ export function markdownToAgentDefinition(
tools: markdown.tools,
}
: undefined,
policy: markdown.policy,
mcpServers: markdown.mcp_servers,
inputConfig,
metadata,
};
@@ -550,6 +550,104 @@ describe('LocalAgentExecutor', () => {
getToolSpy.mockRestore();
});
it('should support scoped policy and unique toolsets', async () => {
const toolToAllow = 'ls';
const toolToBlock = READ_FILE_TOOL_NAME;
const definition = createTestDefinition([toolToAllow, toolToBlock]);
definition.policy = {
tools: {
exclude: [toolToBlock],
},
};
const executor = await LocalAgentExecutor.create(
definition,
mockConfig,
onActivity,
);
const agentRegistry = executor['toolRegistry'];
// Tool explicitly allowed in definition but BLOCKED by policy should NOT be active
expect(agentRegistry.getTool(toolToAllow)).toBeDefined();
expect(agentRegistry.getTool(toolToBlock)).toBeUndefined();
});
it('should incorporate ADMIN policies from parent context into scoped policy engine', async () => {
const adminToolToBlock = 'admin_blocked_tool';
const userToolToBlock = 'user_blocked_tool';
// Mock parent policy engine with admin and user rules
const parentPolicyEngine = mockConfig.getPolicyEngine();
vi.spyOn(parentPolicyEngine, 'getRules').mockReturnValue([
{
toolName: adminToolToBlock,
decision: ApprovalMode.DEFAULT, // Use actual enum value if possible, or cast appropriately
priority: 5.1, // Admin tier
source: 'Admin Policy',
},
{
toolName: userToolToBlock,
decision: ApprovalMode.DEFAULT,
priority: 4.1, // User tier
source: 'User Policy',
},
] as unknown as PolicyRule[]);
vi.spyOn(parentPolicyEngine, 'getCheckers').mockReturnValue([]);
// Create a subagent definition that has its own policy
const definition = createTestDefinition([
adminToolToBlock,
userToolToBlock,
LS_TOOL_NAME,
]);
definition.policy = {
tools: {
allowed: [userToolToBlock], // Subagent tries to allow what user blocked
},
};
const executor = await LocalAgentExecutor.create(
definition,
mockConfig,
onActivity,
);
const scopedPolicyEngine = executor['runtimeContext'].getPolicyEngine();
const rules = scopedPolicyEngine.getRules();
// Should have incorporated the ADMIN rule
expect(rules).toContainEqual(
expect.objectContaining({
toolName: adminToolToBlock,
priority: 5.1,
}),
);
// Should NOT have incorporated the USER rule
expect(rules).not.toContainEqual(
expect.objectContaining({
toolName: userToolToBlock,
priority: 4.1,
}),
);
// Verify effective decisions
const adminCheck = await scopedPolicyEngine.check(
{ name: adminToolToBlock, args: {} },
undefined,
);
expect(adminCheck.decision).toBe('deny');
const userCheck = await scopedPolicyEngine.check(
{ name: userToolToBlock, args: {} },
undefined,
);
// User block was NOT inherited, and subagent policy allowed it.
expect(userCheck.decision).toBe('allow');
});
});
describe('run (Execution Loop and Logic)', () => {
@@ -2451,4 +2549,59 @@ describe('LocalAgentExecutor', () => {
expect(mockSetHistory).toHaveBeenCalledWith(compressedHistory);
});
});
describe('Isolated Tool Discovery and Shadowing', () => {
it('should allow subagent-specific MCP tools to shadow global tools in the isolated registry', async () => {
const toolName = 'shadowed_tool';
const globalTool = new MockTool({ name: toolName, description: 'Global Version' });
parentToolRegistry.registerTool(globalTool);
const subagentMcpTool = {
tool: vi.fn(),
callTool: vi.fn(),
} as unknown as CallableTool;
const mcpTool = new DiscoveredMCPTool(
subagentMcpTool,
'private-server',
toolName,
'Subagent Version',
{},
mockConfig.getMessageBus(),
);
// Mock McpClientManager to register our shadowing tool
const definition = createTestDefinition();
definition.mcpServers = {
'private-server': { command: 'node', args: ['server.js'] }
};
// We need to mock the McpClientManager's behavior
const McpClientManagerModule = await import('../tools/mcp-client-manager.js');
vi.spyOn(McpClientManagerModule.McpClientManager.prototype, 'maybeDiscoverMcpServer')
.mockImplementation(async function(this: unknown) {
// Manually register the tool into the registry provided to the manager
const manager = this as unknown as { toolRegistry: ToolRegistry };
manager.toolRegistry.registerTool(mcpTool);
});
const executor = await LocalAgentExecutor.create(
definition,
mockConfig,
onActivity,
);
const agentRegistry = executor['toolRegistry'];
// The tool in the agent's registry should be the subagent's version
const tool = agentRegistry.getTool(toolName);
expect(tool).toBeDefined();
expect(tool?.description).toContain('Subagent Version');
expect(tool).not.toBe(globalTool);
// The global registry should remain unchanged
expect(parentToolRegistry.getTool(toolName)).toBe(globalTool);
expect(parentToolRegistry.getTool(toolName)?.description).toContain('Global Version');
});
});
});
+74 -2
View File
@@ -20,6 +20,9 @@ import {
DiscoveredMCPTool,
MCP_QUALIFIED_NAME_SEPARATOR,
} from '../tools/mcp-tool.js';
import { PolicyEngine } from '../policy/policy-engine.js';
import { createPolicyEngineConfig } from '../policy/config.js';
import { McpClientManager } from '../tools/mcp-client-manager.js';
import { CompressionStatus } from '../core/turn.js';
import { type ToolCallRequestInfo } from '../scheduler/types.js';
import { ChatCompressionService } from '../services/chatCompressionService.js';
@@ -118,12 +121,81 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
runtimeContext: Config,
onActivity?: ActivityCallback,
): Promise<LocalAgentExecutor<TOutput>> {
const parentToolRegistry = runtimeContext.getToolRegistry();
// Create an isolated tool registry for this agent instance.
const agentToolRegistry = new ToolRegistry(
runtimeContext,
runtimeContext.getMessageBus(),
);
const parentToolRegistry = runtimeContext.getToolRegistry();
// Create a scoped configuration if subagent has private policies or MCP servers.
let agentContext = runtimeContext;
if (definition.policy || definition.mcpServers) {
const parentPolicyEngine = runtimeContext.getPolicyEngine();
// Admin policies have priority >= ADMIN_POLICY_TIER (5).
const adminRules = parentPolicyEngine
.getRules()
.filter((r) => (r.priority ?? 0) >= 5);
const adminCheckers = parentPolicyEngine
.getCheckers()
.filter((c) => (c.priority ?? 0) >= 5);
const policyConfig = definition.policy
? await createPolicyEngineConfig(
definition.policy,
runtimeContext.getApprovalMode(),
undefined, // defaultPoliciesDir
adminRules,
adminCheckers,
)
: {
rules: adminRules,
checkers: adminCheckers,
approvalMode: runtimeContext.getApprovalMode(),
};
const scopedPolicyEngine = new PolicyEngine(
policyConfig,
runtimeContext.getCheckerRunner(),
);
// Populate scoped registry with parent's tools (including built-ins).
// If a subagent has private MCP servers, tools from those servers will
// be registered during discovery and will overwrite any global tools
// with the same name in this isolated registry.
for (const tool of parentToolRegistry.getAllKnownTools()) {
agentToolRegistry.registerTool(tool);
}
let scopedMcpManager: McpClientManager | undefined;
if (definition.mcpServers) {
scopedMcpManager = new McpClientManager(
runtimeContext.getClientVersion(),
agentToolRegistry,
runtimeContext, // Use parent context for some services, but we will scope it soon
);
}
agentContext = runtimeContext.createScopedConfig({
policyEngine: scopedPolicyEngine,
toolRegistry: agentToolRegistry,
mcpClientManager: scopedMcpManager,
});
// Update registry and mcp manager to use the scoped context
agentToolRegistry.setConfig(agentContext);
if (scopedMcpManager) {
scopedMcpManager.setConfig(agentContext);
// Discover and register subagent-specific MCP tools
for (const [name, config] of Object.entries(definition.mcpServers!)) {
await scopedMcpManager.maybeDiscoverMcpServer(name, config);
}
}
}
const allAgentNames = new Set(
runtimeContext.getAgentRegistry().getAllAgentNames(),
);
@@ -186,7 +258,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
return new LocalAgentExecutor(
definition,
runtimeContext,
agentContext,
agentToolRegistry,
parentPromptId,
parentCallId,
+12
View File
@@ -14,6 +14,8 @@ import { type z } from 'zod';
import type { ModelConfig } from '../services/modelConfigService.js';
import type { AnySchema } from 'ajv';
import type { A2AAuthConfig } from './auth-provider/types.js';
import type { PolicySettings } from '../policy/types.js';
import type { MCPServerConfig } from '../config/config.js';
/**
* Describes the possible termination modes for an agent.
@@ -104,6 +106,16 @@ export interface LocalAgentDefinition<
// Optional configs
toolConfig?: ToolConfig;
/**
* Scoped policy settings for this agent.
*/
policy?: PolicySettings;
/**
* MCP servers private to this agent.
*/
mcpServers?: Record<string, MCPServerConfig>;
/**
* An optional function to process the raw output from the agent's final tool
* call into a string format.
+63 -6
View File
@@ -419,6 +419,12 @@ export class MCPServerConfig {
readonly targetAudience?: string,
/* targetServiceAccount format: <service-account-name>@<project-num>.iam.gserviceaccount.com */
readonly targetServiceAccount?: string,
/**
* Visibility of the MCP server.
* 'public' (default): available to all agents.
* 'private': hidden from the primary agent, available only to specific subagents.
*/
readonly visibility?: 'public' | 'private',
) {}
}
@@ -583,7 +589,9 @@ export interface ConfigParameters {
export class Config implements McpContext {
private toolRegistry!: ToolRegistry;
private _scopedToolRegistry?: ToolRegistry;
private mcpClientManager?: McpClientManager;
private _scopedMcpClientManager?: McpClientManager;
private allowedMcpServers: string[];
private blockedMcpServers: string[];
private allowedEnvironmentVariables: string[];
@@ -723,6 +731,7 @@ export class Config implements McpContext {
private readonly useWriteTodos: boolean;
private readonly messageBus: MessageBus;
private readonly policyEngine: PolicyEngine;
private _scopedPolicyEngine?: PolicyEngine;
private policyUpdateConfirmationRequest:
| PolicyUpdateConfirmationRequest
| undefined;
@@ -1314,10 +1323,18 @@ export class Config implements McpContext {
return this.sessionId;
}
getClientVersion(): string {
return this.clientVersion;
}
setSessionId(sessionId: string): void {
this.sessionId = sessionId;
}
getCheckerRunner(): CheckerRunner | undefined {
return (this as any).checkerRunner;
}
setTerminalBackground(terminalBackground: string | undefined): void {
this.terminalBackground = terminalBackground;
}
@@ -1567,7 +1584,15 @@ export class Config implements McpContext {
}
getToolRegistry(): ToolRegistry {
return this.toolRegistry;
return this._scopedToolRegistry ?? this.toolRegistry;
}
getMcpClientManager(): McpClientManager | undefined {
return this._scopedMcpClientManager ?? this.mcpClientManager;
}
setMcpClientManager(manager: McpClientManager): void {
this.mcpClientManager = manager;
}
getPromptRegistry(): PromptRegistry {
@@ -1723,7 +1748,7 @@ export class Config implements McpContext {
}
}
const policyExclusions = this.policyEngine.getExcludedTools(
const policyExclusions = this.getPolicyEngine().getExcludedTools(
toolMetadata,
allToolNames,
);
@@ -1767,9 +1792,6 @@ export class Config implements McpContext {
return this.extensionsEnabled;
}
getMcpClientManager(): McpClientManager | undefined {
return this.mcpClientManager;
}
setUserInteractedWithMcp(): void {
this.mcpClientManager?.setUserInteractedWithMcp();
@@ -2695,7 +2717,7 @@ export class Config implements McpContext {
}
getPolicyEngine(): PolicyEngine {
return this.policyEngine;
return this._scopedPolicyEngine ?? this.policyEngine;
}
getEnableHooks(): boolean {
@@ -2986,6 +3008,41 @@ export class Config implements McpContext {
}
};
/**
* Creates a scoped copy of this configuration with overrides for policy and tools.
* Scoped configs are used by subagents to maintain isolation from the primary agent.
* This uses prototype-based shadowing for overrides while maintaining access to global state.
*/
createScopedConfig(overrides: {
policyEngine?: PolicyEngine;
toolRegistry?: ToolRegistry;
mcpClientManager?: McpClientManager;
}): Config {
const scoped = Object.create(this) as unknown as Config;
// Define properties explicitly to ensure they shadow the base class properties
Object.defineProperties(scoped, {
_scopedPolicyEngine: {
value: overrides.policyEngine,
writable: true,
configurable: true,
enumerable: true,
},
_scopedToolRegistry: {
value: overrides.toolRegistry,
writable: true,
configurable: true,
enumerable: true,
},
_scopedMcpClientManager: {
value: overrides.mcpClientManager,
writable: true,
configurable: true,
enumerable: true,
},
});
return scoped;
}
/**
* Disposes of resources and removes event listeners.
*/
+4 -2
View File
@@ -249,6 +249,8 @@ export async function createPolicyEngineConfig(
settings: PolicySettings,
approvalMode: ApprovalMode,
defaultPoliciesDir?: string,
baseRules: PolicyRule[] = [],
baseCheckers: SafetyCheckerRule[] = [],
): Promise<PolicyEngineConfig> {
const policyDirs = getPolicyDirectories(
defaultPoliciesDir,
@@ -297,8 +299,8 @@ export async function createPolicyEngineConfig(
}
}
const rules: PolicyRule[] = [...tomlRules];
const checkers = [...tomlCheckers];
const rules: PolicyRule[] = [...baseRules, ...tomlRules];
const checkers = [...baseCheckers, ...tomlCheckers];
// Priority system for policy rules:
+46
View File
@@ -0,0 +1,46 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { z } from 'zod';
/**
* Zod schema for MCPServerConfig.
*/
export const MCPServerConfigSchema = z.object({
command: z.string().optional(),
args: z.array(z.string()).optional(),
env: z.record(z.string()).optional(),
trust: z.boolean().optional(),
alwaysAllowTools: z.array(z.string()).optional(),
includeTools: z.array(z.string()).optional(),
excludeTools: z.array(z.string()).optional(),
targetAudience: z.string().optional(),
targetServiceAccount: z.string().optional(),
visibility: z.enum(['public', 'private']).optional(),
});
/**
* Zod schema for PolicySettings.
*/
export const PolicySettingsSchema = z.object({
mcp: z
.object({
excluded: z.array(z.string()).optional(),
allowed: z.array(z.string()).optional(),
})
.optional(),
tools: z
.object({
exclude: z.array(z.string()).optional(),
allowed: z.array(z.string()).optional(),
})
.optional(),
mcpServers: z.record(z.object({ trust: z.boolean().optional() })).optional(),
policyPaths: z.array(z.string()).optional(),
workspacePoliciesDir: z.string().optional(),
});
export const MCPServersConfigSchema = z.record(MCPServerConfigSchema);
@@ -76,6 +76,14 @@ export class McpClientManager {
this.eventEmitter = eventEmitter;
}
/**
* Updates the configuration used by the MCP client manager.
* This is used when creating scoped managers for subagents.
*/
setConfig(config: Config): void {
this.cliConfig = config;
}
setUserInteractedWithMcp() {
this.userInteractedWithMcp = true;
}
+15
View File
@@ -206,6 +206,14 @@ export class ToolRegistry {
this.messageBus = messageBus;
}
/**
* Updates the configuration used by the tool registry.
* This is used when creating scoped registries for subagents.
*/
setConfig(config: Config): void {
this.config = config;
}
getMessageBus(): MessageBus {
return this.messageBus;
}
@@ -232,6 +240,13 @@ export class ToolRegistry {
this.allKnownTools.set(tool.name, tool);
}
/**
* Returns all known tools, including inactive ones.
*/
getAllKnownTools(): AnyDeclarativeTool[] {
return Array.from(this.allKnownTools.values());
}
/**
* Unregisters a tool definition by name.
*