mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-08-09 00:16:57 -07:00
fix(core): improve team isolation, policy, and model config
- Implement dynamic agent registration/unregistration in TeamRegistry - Only promote active team agents to the global AgentRegistry - Automatically ALLOW team agents in policy engine (isTeamAgent metadata) - Fix ModelNotFoundError for external agents by registering default model configs - Update Config to refresh subagent tools when active team changes - Resolve bug where all discovered team agents were available to orchestrator
This commit is contained in:
@@ -1424,8 +1424,8 @@ Logging in with Google... Restarting Gemini CLI to continue.
|
||||
const { handleInput: vimHandleInput } = useVim(buffer, handleFinalSubmit);
|
||||
|
||||
const handleTeamSelect = useCallback(
|
||||
(teamName: string | undefined) => {
|
||||
config.setActiveTeam(teamName);
|
||||
async (teamName: string | undefined) => {
|
||||
await config.setActiveTeam(teamName);
|
||||
settings.setValue(SettingScope.Workspace, 'general.activeTeam', teamName);
|
||||
setIsTeamSelectionActive(false);
|
||||
refreshStatic();
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
|
||||
import type React from 'react';
|
||||
import { useState, useMemo, useCallback } from 'react';
|
||||
import { Box, Text , useStdin } from 'ink';
|
||||
import { Box, Text, useStdin } from 'ink';
|
||||
import {
|
||||
type ScaffoldTeamAgent,
|
||||
scaffoldTeam,
|
||||
@@ -104,7 +104,7 @@ export function TeamCreatorWizard({
|
||||
() =>
|
||||
config
|
||||
?.getAgentRegistry()
|
||||
.getAllDefinitions()
|
||||
.getAllDiscoveredDefinitions()
|
||||
.filter((a) => a.kind === 'local') || [],
|
||||
[config],
|
||||
);
|
||||
@@ -295,7 +295,7 @@ export function TeamCreatorWizard({
|
||||
(key: Key) => {
|
||||
if (step === 'success') {
|
||||
if (key.sequence?.toLowerCase() === 'y') {
|
||||
handleTeamSelect(teamName);
|
||||
void handleTeamSelect(teamName);
|
||||
}
|
||||
onComplete();
|
||||
return true;
|
||||
|
||||
@@ -312,6 +312,33 @@ export class AgentRegistry {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Unregisters an agent definition from the active agents list.
|
||||
* It remains in the discovered definitions list.
|
||||
*/
|
||||
unregisterAgent(name: string): void {
|
||||
this.agents.delete(name);
|
||||
|
||||
// Unregister model configs
|
||||
this.config.modelConfigService.unregisterRuntimeModelConfig(
|
||||
`${name}-config`,
|
||||
);
|
||||
this.config.modelConfigService.unregisterRuntimeModelOverridesByScope(name);
|
||||
|
||||
// Remove policies
|
||||
const policyEngine = this.config.getPolicyEngine();
|
||||
if (policyEngine) {
|
||||
policyEngine.removeRulesForTool(name, 'AgentRegistry (Dynamic)');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers an agent definition for discovery only.
|
||||
*/
|
||||
registerDiscoveredAgent(definition: AgentDefinition): void {
|
||||
this.allDefinitions.set(definition.name, definition);
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers an agent definition. If an agent with the same name exists,
|
||||
* it will be overwritten, respecting the precedence established by the
|
||||
@@ -362,6 +389,16 @@ export class AgentRegistry {
|
||||
}
|
||||
|
||||
this.agents.set(definition.name, definition);
|
||||
|
||||
// Register default model config for external agents
|
||||
this.registerModelConfigs({
|
||||
...definition,
|
||||
kind: 'local',
|
||||
modelConfig: { model: 'inherit' },
|
||||
runConfig: {},
|
||||
promptConfig: {},
|
||||
});
|
||||
|
||||
this.addAgentPolicy(definition);
|
||||
}
|
||||
|
||||
@@ -432,7 +469,7 @@ export class AgentRegistry {
|
||||
policyEngine.addRule({
|
||||
toolName: definition.name,
|
||||
decision:
|
||||
definition.kind === 'local'
|
||||
definition.kind === 'local' || definition.metadata?.isTeamAgent
|
||||
? PolicyDecision.ALLOW
|
||||
: PolicyDecision.ASK_USER,
|
||||
priority: PRIORITY_SUBAGENT_TOOL,
|
||||
@@ -730,6 +767,13 @@ export class AgentRegistry {
|
||||
return Array.from(this.agents.values());
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all discovered agent definitions.
|
||||
*/
|
||||
getAllDiscoveredDefinitions(): AgentDefinition[] {
|
||||
return Array.from(this.allDefinitions.values());
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a list of all registered agent names.
|
||||
*/
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { type TeamDefinition } from './types.js';
|
||||
import { type TeamDefinition, type AgentDefinition } from './types.js';
|
||||
import { type Config } from '../config/config.js';
|
||||
import { Storage } from '../config/storage.js';
|
||||
import { type AgentRegistry } from './registry.js';
|
||||
@@ -79,37 +79,14 @@ export class TeamRegistry {
|
||||
coreEvents.emitFeedback('error', `Team loading error: ${error.message}`);
|
||||
}
|
||||
|
||||
const registrationPromises: Array<Promise<void>> = [];
|
||||
|
||||
for (const team of result.teams) {
|
||||
this.teams.set(team.name, team);
|
||||
|
||||
// Register team agents in the global AgentRegistry so they are available as SubagentTools
|
||||
// Record team agents for discovery (e.g. in the Team Creator)
|
||||
for (const agent of team.agents) {
|
||||
const descriptionOverride = `MANDATORY for ${agent.displayName} tasks: ${agent.description} (Team Agent: ${team.displayName}). You MUST delegate all ${agent.displayName} tasks to this agent.`;
|
||||
|
||||
// We wrap the agent definition to provide the description override
|
||||
const wrappedAgent = {
|
||||
...agent,
|
||||
description: descriptionOverride,
|
||||
};
|
||||
|
||||
registrationPromises.push(
|
||||
this.agentRegistry.registerAgent(wrappedAgent).catch((e) => {
|
||||
debugLogger.warn(
|
||||
`[TeamRegistry] Error registering agent "${agent.name}" from team "${team.name}":`,
|
||||
e,
|
||||
);
|
||||
coreEvents.emitFeedback(
|
||||
'error',
|
||||
`Error registering agent "${agent.name}" from team "${team.name}": ${e instanceof Error ? e.message : String(e)}`,
|
||||
);
|
||||
}),
|
||||
);
|
||||
this.agentRegistry.registerDiscoveredAgent(agent);
|
||||
}
|
||||
}
|
||||
|
||||
await Promise.allSettled(registrationPromises);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -124,19 +101,70 @@ export class TeamRegistry {
|
||||
* @param name The slug (name) of the team to activate, or undefined to clear.
|
||||
* @throws Error if the team is not found and name is provided.
|
||||
*/
|
||||
setActiveTeam(name: string | undefined): void {
|
||||
async setActiveTeam(name: string | undefined): Promise<void> {
|
||||
const previousTeam = this.getActiveTeam();
|
||||
|
||||
if (name === undefined) {
|
||||
this.activeTeamName = undefined;
|
||||
if (previousTeam) {
|
||||
this.unregisterTeamAgents(previousTeam);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.teams.has(name)) {
|
||||
if (previousTeam) {
|
||||
this.unregisterTeamAgents(previousTeam);
|
||||
}
|
||||
this.activeTeamName = name;
|
||||
const newTeam = this.getActiveTeam();
|
||||
if (newTeam) {
|
||||
await this.registerTeamAgents(newTeam);
|
||||
}
|
||||
} else {
|
||||
throw new Error(`Team not found: ${name}`);
|
||||
}
|
||||
}
|
||||
|
||||
private async registerTeamAgents(team: TeamDefinition): Promise<void> {
|
||||
const registrationPromises: Array<Promise<void>> = [];
|
||||
|
||||
for (const agent of team.agents) {
|
||||
const descriptionOverride = `MANDATORY for ${agent.displayName} tasks: ${agent.description} (Team Agent: ${team.displayName}). You MUST delegate all ${agent.displayName} tasks to this agent.`;
|
||||
|
||||
// We wrap the agent definition to provide the description override
|
||||
const wrappedAgent: AgentDefinition = {
|
||||
...agent,
|
||||
description: descriptionOverride,
|
||||
metadata: {
|
||||
...agent.metadata,
|
||||
isTeamAgent: true,
|
||||
},
|
||||
};
|
||||
|
||||
registrationPromises.push(
|
||||
this.agentRegistry.registerAgent(wrappedAgent).catch((e) => {
|
||||
debugLogger.warn(
|
||||
`[TeamRegistry] Error registering agent "${agent.name}" from team "${team.name}":`,
|
||||
e,
|
||||
);
|
||||
coreEvents.emitFeedback(
|
||||
'error',
|
||||
`Error registering agent "${agent.name}" from team "${team.name}": ${e instanceof Error ? e.message : String(e)}`,
|
||||
);
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
await Promise.allSettled(registrationPromises);
|
||||
}
|
||||
|
||||
private unregisterTeamAgents(team: TeamDefinition): void {
|
||||
for (const agent of team.agents) {
|
||||
this.agentRegistry.unregisterAgent(agent.name);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the currently active team definition, if any.
|
||||
*/
|
||||
|
||||
@@ -199,6 +199,7 @@ export interface BaseAgentDefinition<
|
||||
metadata?: {
|
||||
hash?: string;
|
||||
filePath?: string;
|
||||
isTeamAgent?: boolean;
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -1438,13 +1438,14 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
|
||||
if (this._params.activeTeam) {
|
||||
try {
|
||||
this.teamRegistry.setActiveTeam(this._params.activeTeam);
|
||||
await this.teamRegistry.setActiveTeam(this._params.activeTeam);
|
||||
} catch (_e) {
|
||||
// Ignore if team not found (might have been deleted or is project-specific)
|
||||
}
|
||||
}
|
||||
|
||||
coreEvents.on(CoreEvent.AgentsRefreshed, this.onAgentsRefreshed);
|
||||
coreEvents.on(CoreEvent.ActiveTeamChanged, this.onActiveTeamChanged);
|
||||
|
||||
this._toolRegistry = await this.createToolRegistry();
|
||||
discoverToolsHandle?.end();
|
||||
@@ -2047,11 +2048,17 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
return this.teamRegistry.getActiveTeam();
|
||||
}
|
||||
|
||||
setActiveTeam(name: string | undefined): void {
|
||||
this.teamRegistry.setActiveTeam(name);
|
||||
async setActiveTeam(name: string | undefined): Promise<void> {
|
||||
await this.teamRegistry.setActiveTeam(name);
|
||||
coreEvents.emitActiveTeamChanged(name);
|
||||
}
|
||||
|
||||
private onActiveTeamChanged = () => {
|
||||
if (this._toolRegistry) {
|
||||
this.registerSubAgentTools(this._toolRegistry);
|
||||
}
|
||||
};
|
||||
|
||||
getAcknowledgedAgentsService(): AcknowledgedAgentsService {
|
||||
return this.acknowledgedAgentsService;
|
||||
}
|
||||
@@ -3603,36 +3610,30 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
*/
|
||||
private registerSubAgentTools(registry: ToolRegistry): void {
|
||||
const agentsOverrides = this.getAgentsSettings().overrides ?? {};
|
||||
const discoveredDefinitions =
|
||||
this.agentRegistry.getAllDiscoveredAgentNames();
|
||||
|
||||
// First, unregister any agents that are now disabled
|
||||
for (const agentName of discoveredDefinitions) {
|
||||
if (
|
||||
!this.isAgentsEnabled() ||
|
||||
agentsOverrides[agentName]?.enabled === false
|
||||
) {
|
||||
const tool = registry.getTool(agentName);
|
||||
if (tool instanceof SubagentTool) {
|
||||
registry.unregisterTool(agentName);
|
||||
}
|
||||
// First, unregister ALL current SubagentTools to ensure a clean state
|
||||
const allTools = registry.getAllTools();
|
||||
for (const tool of allTools) {
|
||||
if (tool instanceof SubagentTool) {
|
||||
registry.unregisterTool(tool.name);
|
||||
}
|
||||
}
|
||||
|
||||
const discoveredNames = this.agentRegistry.getAllDiscoveredAgentNames();
|
||||
if (!this.isAgentsEnabled()) {
|
||||
return;
|
||||
}
|
||||
|
||||
const activeAgentNames = this.agentRegistry.getAllAgentNames();
|
||||
const activeTeam = this.teamRegistry.getActiveTeam();
|
||||
const teamAgentNames = new Set(activeTeam?.agents.map((a) => a.name) ?? []);
|
||||
|
||||
for (const agentName of discoveredNames) {
|
||||
const definition = this.agentRegistry.getDiscoveredDefinition(agentName);
|
||||
for (const agentName of activeAgentNames) {
|
||||
const definition = this.agentRegistry.getDefinition(agentName);
|
||||
if (!definition) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
if (
|
||||
!this.isAgentsEnabled() ||
|
||||
agentsOverrides[definition.name]?.enabled === false
|
||||
) {
|
||||
if (agentsOverrides[definition.name]?.enabled === false) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -333,10 +333,22 @@ export class ModelConfigService {
|
||||
this.runtimeAliases[aliasName] = alias;
|
||||
}
|
||||
|
||||
unregisterRuntimeModelConfig(aliasName: string): void {
|
||||
delete this.runtimeAliases[aliasName];
|
||||
}
|
||||
|
||||
registerRuntimeModelOverride(override: ModelConfigOverride): void {
|
||||
this.runtimeOverrides.push(override);
|
||||
}
|
||||
|
||||
unregisterRuntimeModelOverridesByScope(scope: string): void {
|
||||
for (let i = this.runtimeOverrides.length - 1; i >= 0; i--) {
|
||||
if (this.runtimeOverrides[i].match.overrideScope === scope) {
|
||||
this.runtimeOverrides.splice(i, 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a model configuration by merging settings from aliases and applying overrides.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user