From 8cda688fe24de99a0add72d70ed54c19c2e9f5c0 Mon Sep 17 00:00:00 2001 From: Adam Weidman <65992621+adamfweidman@users.noreply.github.com> Date: Tue, 12 May 2026 21:33:12 -0400 Subject: [PATCH] feat(core): change agent registration to first-wins and prioritize project (#26953) --- packages/core/src/agents/registry.test.ts | 34 ++++--------- packages/core/src/agents/registry.ts | 61 +++++++++++++---------- 2 files changed, 46 insertions(+), 49 deletions(-) diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 7618440957..7f53972b58 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -250,11 +250,11 @@ describe('AgentRegistry', () => { }; vi.mocked(tomlLoader.loadAgentsFromDirectory) - .mockResolvedValueOnce({ agents: [userAgent], errors: [] }) // User dir .mockResolvedValueOnce({ agents: [projectAgent, uniqueProjectAgent], errors: [], - }); // Project dir + }) // Project dir + .mockResolvedValueOnce({ agents: [userAgent], errors: [] }); // User dir await registry.initialize(); @@ -1011,7 +1011,7 @@ describe('AgentRegistry', () => { ); }); - it('should overwrite an existing agent definition', async () => { + it('should NOT overwrite an existing agent definition', async () => { await registry.testRegisterAgent(MOCK_AGENT_V1); expect(registry.getDefinition('MockAgent')?.description).toBe( 'Mock Description V1', @@ -1019,36 +1019,22 @@ describe('AgentRegistry', () => { await registry.testRegisterAgent(MOCK_AGENT_V2); expect(registry.getDefinition('MockAgent')?.description).toBe( - 'Mock Description V2 (Updated)', + 'Mock Description V1', ); expect(registry.getAllDefinitions()).toHaveLength(1); }); - it('should log overwrites when in debug mode', async () => { - const debugConfig = makeMockedConfig({ debugMode: true }); - const debugRegistry = new TestableAgentRegistry(debugConfig); - const debugLogSpy = vi - .spyOn(debugLogger, 'log') - .mockImplementation(() => {}); - - await debugRegistry.testRegisterAgent(MOCK_AGENT_V1); - await debugRegistry.testRegisterAgent(MOCK_AGENT_V2); - - expect(debugLogSpy).toHaveBeenCalledWith( - `[AgentRegistry] Overriding agent 'MockAgent'`, - ); - }); - - it('should not log overwrites when not in debug mode', async () => { - const debugLogSpy = vi - .spyOn(debugLogger, 'log') + it('should emit warning on duplicate agent definition', async () => { + const feedbackSpy = vi + .spyOn(coreEvents, 'emitFeedback') .mockImplementation(() => {}); await registry.testRegisterAgent(MOCK_AGENT_V1); await registry.testRegisterAgent(MOCK_AGENT_V2); - expect(debugLogSpy).not.toHaveBeenCalledWith( - `[AgentRegistry] Overriding agent 'MockAgent'`, + expect(feedbackSpy).toHaveBeenCalledWith( + 'warning', + expect.stringContaining("Duplicate agent name 'MockAgent' detected"), ); }); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index b9d434e4c7..92405a0c8f 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -169,31 +169,6 @@ export class AgentRegistry { return; } - // Load user-level agents: ~/.gemini/agents/ - const userAgentsDir = Storage.getUserAgentsDir(); - const userAgents = await loadAgentsFromDirectory(userAgentsDir); - for (const error of userAgents.errors) { - debugLogger.warn( - `[AgentRegistry] Error loading user agent: ${error.message}`, - ); - const msg = `Agent loading error: ${error.message}`; - errors?.push(msg); - coreEvents.emitFeedback('error', msg); - } - await Promise.allSettled( - userAgents.agents.map(async (agent) => { - try { - this.ensureRemoteAgentHash(agent); - await this.registerAgent(agent, errors); - } catch (e) { - const msg = `Error registering user agent "${agent.name}": ${e instanceof Error ? e.message : String(e)}`; - debugLogger.warn(`[AgentRegistry] ${msg}`, e); - errors?.push(msg); - coreEvents.emitFeedback('error', msg); - } - }), - ); - // Load project-level agents: .gemini/agents/ (relative to Project Root) const folderTrustEnabled = this.config.getFolderTrust(); const isTrustedFolder = this.config.isTrustedFolder(); @@ -256,6 +231,31 @@ export class AgentRegistry { ); } + // Load user-level agents: ~/.gemini/agents/ + const userAgentsDir = Storage.getUserAgentsDir(); + const userAgents = await loadAgentsFromDirectory(userAgentsDir); + for (const error of userAgents.errors) { + debugLogger.warn( + `[AgentRegistry] Error loading user agent: ${error.message}`, + ); + const msg = `Agent loading error: ${error.message}`; + errors?.push(msg); + coreEvents.emitFeedback('error', msg); + } + await Promise.allSettled( + userAgents.agents.map(async (agent) => { + try { + this.ensureRemoteAgentHash(agent); + await this.registerAgent(agent, errors); + } catch (e) { + const msg = `Error registering user agent "${agent.name}": ${e instanceof Error ? e.message : String(e)}`; + debugLogger.warn(`[AgentRegistry] ${msg}`, e); + errors?.push(msg); + coreEvents.emitFeedback('error', msg); + } + }), + ); + // Load agents from extensions for (const extension of this.config.getExtensions()) { if (extension.isActive && extension.agents) { @@ -336,6 +336,17 @@ export class AgentRegistry { definition: AgentDefinition, errors?: string[], ): Promise { + const existing = this.agents.get(definition.name); + if (existing && existing !== definition) { + coreEvents.emitFeedback( + 'warning', + `Duplicate agent name '${definition.name}' detected. ` + + `The later definition will be ignored. ` + + `Rename one of the agents to avoid this conflict.`, + ); + return; + } + if (definition.kind === 'local') { this.registerLocalAgent(definition); } else if (definition.kind === 'remote') {