diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 7618440957..f57ce9f5d6 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -1011,44 +1011,57 @@ describe('AgentRegistry', () => { ); }); - it('should overwrite an existing agent definition', async () => { + it('should override an existing agent but warn on duplicate name', async () => { await registry.testRegisterAgent(MOCK_AGENT_V1); expect(registry.getDefinition('MockAgent')?.description).toBe( 'Mock Description V1', ); + const feedbackSpy = vi.spyOn(coreEvents, 'emitFeedback'); await registry.testRegisterAgent(MOCK_AGENT_V2); + + // Should override to V2 (preserves existing precedence) expect(registry.getDefinition('MockAgent')?.description).toBe( 'Mock Description V2 (Updated)', ); expect(registry.getAllDefinitions()).toHaveLength(1); + // But should emit a warning about the duplicate + expect(feedbackSpy).toHaveBeenCalledWith( + 'warning', + expect.stringContaining("Duplicate agent name 'MockAgent'"), + ); }); - it('should log overwrites when in debug mode', async () => { + it('should not warn when re-registering the same definition (refresh)', async () => { + await registry.testRegisterAgent(MOCK_AGENT_V1); + expect(registry.getDefinition('MockAgent')?.description).toBe( + 'Mock Description V1', + ); + + // Re-registering the exact same object should succeed without warning + const feedbackSpy = vi.spyOn(coreEvents, 'emitFeedback'); + await registry.testRegisterAgent(MOCK_AGENT_V1); + expect(registry.getDefinition('MockAgent')?.description).toBe( + 'Mock Description V1', + ); + expect(feedbackSpy).not.toHaveBeenCalledWith( + 'warning', + expect.stringContaining('Duplicate'), + ); + }); + + it('should warn on duplicate in debug logs', async () => { const debugConfig = makeMockedConfig({ debugMode: true }); const debugRegistry = new TestableAgentRegistry(debugConfig); - const debugLogSpy = vi - .spyOn(debugLogger, 'log') + const warnSpy = vi + .spyOn(debugLogger, 'warn') .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') - .mockImplementation(() => {}); - - await registry.testRegisterAgent(MOCK_AGENT_V1); - await registry.testRegisterAgent(MOCK_AGENT_V2); - - expect(debugLogSpy).not.toHaveBeenCalledWith( - `[AgentRegistry] Overriding agent 'MockAgent'`, + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Overriding agent'), ); }); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index b9d434e4c7..7264ad7b92 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -328,14 +328,28 @@ export class AgentRegistry { } /** - * Registers an agent definition. If an agent with the same name exists, - * it will be overwritten, respecting the precedence established by the - * initialization order. + * Registers an agent definition. If an agent with the same name already + * exists from a different definition, a warning is emitted to surface + * potential state collision. The new definition still overwrites the old + * one to preserve existing precedence order (user → project → extension). */ protected async registerAgent( 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 override the earlier one. ` + + `Rename one of the agents to avoid this conflict.`, + ); + debugLogger.warn( + `[AgentRegistry] Overriding agent '${definition.name}' — duplicate name from a different definition.`, + ); + } + if (definition.kind === 'local') { this.registerLocalAgent(definition); } else if (definition.kind === 'remote') { diff --git a/packages/core/src/agents/remote-session-invocation.test.ts b/packages/core/src/agents/remote-session-invocation.test.ts index d611dd7399..3811c47627 100644 --- a/packages/core/src/agents/remote-session-invocation.test.ts +++ b/packages/core/src/agents/remote-session-invocation.test.ts @@ -215,7 +215,7 @@ describe('RemoteSessionInvocation', () => { it('should pass initial state from static map to session', async () => { const priorState = { contextId: 'ctx-42', taskId: 'task-42' }; - // Seed the static map before constructing the invocation + // Seed the static map before constructing the invocation (composite key) ( RemoteSessionInvocation as unknown as { sessionState: Map; @@ -257,7 +257,7 @@ describe('RemoteSessionInvocation', () => { abortSignal: new AbortController().signal, }); - // Verify the state was persisted in the static map + // Verify the state was persisted in the static map (composite key) const storedState = ( RemoteSessionInvocation as unknown as { sessionState: Map;