mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-16 14:53:19 -07:00
feat(core): composite sessionState key + duplicate agent name warning
Two changes: 1. RemoteSessionInvocation now uses a composite key (name::targetUrl) for the static sessionState map. This ensures agents with the same name but different endpoints maintain independent A2A state. Falls back to name-only when no URL can be derived. 2. AgentRegistry.registerAgent now emits a visible warning when a different definition tries to register under an existing name. Override still proceeds to preserve existing precedence order (user → project → extension). The warning surfaces potential naming conflicts to users.
This commit is contained in:
committed by
Adam Weidman
parent
ca5d4a2129
commit
db3bdaefcc
@@ -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'),
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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<TOutput extends z.ZodTypeAny>(
|
||||
definition: AgentDefinition<TOutput>,
|
||||
errors?: string[],
|
||||
): Promise<void> {
|
||||
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') {
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
@@ -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<string, { contextId?: string; taskId?: string }>;
|
||||
|
||||
Reference in New Issue
Block a user