feat(agents): implement first-run experience for project-level sub-agents (#17266)

This commit is contained in:
Christian Gunderman
2026-01-26 19:49:32 +00:00
committed by GitHub
parent d745d86af1
commit 2271bbb339
18 changed files with 769 additions and 15 deletions
@@ -0,0 +1,97 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { AcknowledgedAgentsService } from './acknowledgedAgents.js';
import { Storage } from '../config/storage.js';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import * as os from 'node:os';
describe('AcknowledgedAgentsService', () => {
let tempDir: string;
let originalGeminiCliHome: string | undefined;
beforeEach(async () => {
// Create a unique temp directory for each test
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-cli-test-'));
// Override GEMINI_CLI_HOME to point to the temp directory
originalGeminiCliHome = process.env['GEMINI_CLI_HOME'];
process.env['GEMINI_CLI_HOME'] = tempDir;
});
afterEach(async () => {
// Restore environment variable
if (originalGeminiCliHome) {
process.env['GEMINI_CLI_HOME'] = originalGeminiCliHome;
} else {
delete process.env['GEMINI_CLI_HOME'];
}
// Clean up temp directory
await fs.rm(tempDir, { recursive: true, force: true });
});
it('should acknowledge an agent and save to disk', async () => {
const service = new AcknowledgedAgentsService();
const ackPath = Storage.getAcknowledgedAgentsPath();
await service.acknowledge('/project', 'AgentA', 'hash1');
// Verify file exists and content
const content = await fs.readFile(ackPath, 'utf-8');
expect(content).toContain('"AgentA": "hash1"');
});
it('should return true for acknowledged agent', async () => {
const service = new AcknowledgedAgentsService();
await service.acknowledge('/project', 'AgentA', 'hash1');
expect(await service.isAcknowledged('/project', 'AgentA', 'hash1')).toBe(
true,
);
expect(await service.isAcknowledged('/project', 'AgentA', 'hash2')).toBe(
false,
);
expect(await service.isAcknowledged('/project', 'AgentB', 'hash1')).toBe(
false,
);
});
it('should load acknowledged agents from disk', async () => {
const ackPath = Storage.getAcknowledgedAgentsPath();
const data = {
'/project': {
AgentLoaded: 'hashLoaded',
},
};
// Ensure directory exists
await fs.mkdir(path.dirname(ackPath), { recursive: true });
await fs.writeFile(ackPath, JSON.stringify(data), 'utf-8');
const service = new AcknowledgedAgentsService();
expect(
await service.isAcknowledged('/project', 'AgentLoaded', 'hashLoaded'),
).toBe(true);
});
it('should handle load errors gracefully', async () => {
// Create a directory where the file should be to cause a read error (EISDIR)
const ackPath = Storage.getAcknowledgedAgentsPath();
await fs.mkdir(ackPath, { recursive: true });
const service = new AcknowledgedAgentsService();
// Should not throw, and treated as empty
expect(await service.isAcknowledged('/project', 'Agent', 'hash')).toBe(
false,
);
});
});
@@ -0,0 +1,85 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { Storage } from '../config/storage.js';
import { debugLogger } from '../utils/debugLogger.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
export interface AcknowledgedAgentsMap {
// Project Path -> Agent Name -> Agent Hash
[projectPath: string]: {
[agentName: string]: string;
};
}
export class AcknowledgedAgentsService {
private acknowledgedAgents: AcknowledgedAgentsMap = {};
private loaded = false;
async load(): Promise<void> {
if (this.loaded) return;
const filePath = Storage.getAcknowledgedAgentsPath();
try {
const content = await fs.readFile(filePath, 'utf-8');
this.acknowledgedAgents = JSON.parse(content);
} catch (error: unknown) {
if (!isNodeError(error) || error.code !== 'ENOENT') {
debugLogger.error(
'Failed to load acknowledged agents:',
getErrorMessage(error),
);
}
// If file doesn't exist or there's a parsing error, fallback to empty
this.acknowledgedAgents = {};
}
this.loaded = true;
}
async save(): Promise<void> {
const filePath = Storage.getAcknowledgedAgentsPath();
try {
const dir = path.dirname(filePath);
await fs.mkdir(dir, { recursive: true });
await fs.writeFile(
filePath,
JSON.stringify(this.acknowledgedAgents, null, 2),
'utf-8',
);
} catch (error) {
debugLogger.error(
'Failed to save acknowledged agents:',
getErrorMessage(error),
);
}
}
async isAcknowledged(
projectPath: string,
agentName: string,
hash: string,
): Promise<boolean> {
await this.load();
const projectAgents = this.acknowledgedAgents[projectPath];
if (!projectAgents) return false;
return projectAgents[agentName] === hash;
}
async acknowledge(
projectPath: string,
agentName: string,
hash: string,
): Promise<void> {
await this.load();
if (!this.acknowledgedAgents[projectPath]) {
this.acknowledgedAgents[projectPath] = {};
}
this.acknowledgedAgents[projectPath][agentName] = hash;
await this.save();
}
}
+25 -11
View File
@@ -8,10 +8,12 @@ import yaml from 'js-yaml';
import * as fs from 'node:fs/promises';
import { type Dirent } from 'node:fs';
import * as path from 'node:path';
import * as crypto from 'node:crypto';
import { z } from 'zod';
import type { AgentDefinition } from './types.js';
import { isValidToolName } from '../tools/tool-names.js';
import { FRONTMATTER_REGEX } from '../skills/skillLoader.js';
import { getErrorMessage } from '../utils/errors.js';
/**
* DTO for Markdown parsing - represents the structure from frontmatter.
@@ -139,24 +141,30 @@ function formatZodError(error: z.ZodError, context: string): string {
* Parses and validates an agent Markdown file with frontmatter.
*
* @param filePath Path to the Markdown file.
* @param content Optional pre-loaded content of the file.
* @returns An array containing the single parsed agent definition.
* @throws AgentLoadError if parsing or validation fails.
*/
export async function parseAgentMarkdown(
filePath: string,
content?: string,
): Promise<FrontmatterAgentDefinition[]> {
let content: string;
try {
content = await fs.readFile(filePath, 'utf-8');
} catch (error) {
throw new AgentLoadError(
filePath,
`Could not read file: ${(error as Error).message}`,
);
let fileContent: string;
if (content !== undefined) {
fileContent = content;
} else {
try {
fileContent = await fs.readFile(filePath, 'utf-8');
} catch (error) {
throw new AgentLoadError(
filePath,
`Could not read file: ${getErrorMessage(error)}`,
);
}
}
// Split frontmatter and body
const match = content.match(FRONTMATTER_REGEX);
const match = fileContent.match(FRONTMATTER_REGEX);
if (!match) {
throw new AgentLoadError(
filePath,
@@ -229,10 +237,12 @@ export async function parseAgentMarkdown(
* Converts a FrontmatterAgentDefinition DTO to the internal AgentDefinition structure.
*
* @param markdown The parsed Markdown/Frontmatter definition.
* @param metadata Optional metadata including hash and file path.
* @returns The internal AgentDefinition.
*/
export function markdownToAgentDefinition(
markdown: FrontmatterAgentDefinition,
metadata?: { hash?: string; filePath?: string },
): AgentDefinition {
const inputConfig = {
inputSchema: {
@@ -256,6 +266,7 @@ export function markdownToAgentDefinition(
displayName: markdown.display_name,
agentCardUrl: markdown.agent_card_url,
inputConfig,
metadata,
};
}
@@ -288,6 +299,7 @@ export function markdownToAgentDefinition(
}
: undefined,
inputConfig,
metadata,
};
}
@@ -334,9 +346,11 @@ export async function loadAgentsFromDirectory(
for (const entry of files) {
const filePath = path.join(dir, entry.name);
try {
const agentDefs = await parseAgentMarkdown(filePath);
const content = await fs.readFile(filePath, 'utf-8');
const hash = crypto.createHash('sha256').update(content).digest('hex');
const agentDefs = await parseAgentMarkdown(filePath, content);
for (const def of agentDefs) {
const agent = markdownToAgentDefinition(def);
const agent = markdownToAgentDefinition(def, { hash, filePath });
result.agents.push(agent);
}
} catch (error) {
+53
View File
@@ -25,6 +25,7 @@ import { SimpleExtensionLoader } from '../utils/extensionLoader.js';
import type { ConfigParameters } from '../config/config.js';
import type { ToolRegistry } from '../tools/tool-registry.js';
import { ThinkingLevel } from '@google/genai';
import type { AcknowledgedAgentsService } from './acknowledgedAgents.js';
vi.mock('./agentLoader.js', () => ({
loadAgentsFromDirectory: vi
@@ -401,6 +402,58 @@ describe('AgentRegistry', () => {
expect(registry.getDefinition('extension-agent')).toBeUndefined();
});
it('should use agentCardUrl as hash for acknowledgement of remote agents', async () => {
mockConfig = makeMockedConfig({ enableAgents: true });
// Trust the folder so it attempts to load project agents
vi.spyOn(mockConfig, 'isTrustedFolder').mockReturnValue(true);
vi.spyOn(mockConfig, 'getFolderTrust').mockReturnValue(true);
const registry = new TestableAgentRegistry(mockConfig);
const remoteAgent: AgentDefinition = {
kind: 'remote',
name: 'RemoteAgent',
description: 'A remote agent',
agentCardUrl: 'https://example.com/card',
inputConfig: { inputSchema: { type: 'object' } },
metadata: { hash: 'file-hash', filePath: 'path/to/file.md' },
};
vi.mocked(tomlLoader.loadAgentsFromDirectory).mockResolvedValue({
agents: [remoteAgent],
errors: [],
});
const ackService = {
isAcknowledged: vi.fn().mockResolvedValue(true),
acknowledge: vi.fn(),
};
vi.spyOn(mockConfig, 'getAcknowledgedAgentsService').mockReturnValue(
ackService as unknown as AcknowledgedAgentsService,
);
// Mock A2AClientManager to avoid network calls
vi.mocked(A2AClientManager.getInstance).mockReturnValue({
loadAgent: vi.fn().mockResolvedValue({ name: 'RemoteAgent' }),
clearCache: vi.fn(),
} as unknown as A2AClientManager);
await registry.initialize();
// Verify ackService was called with the URL, not the file hash
expect(ackService.isAcknowledged).toHaveBeenCalledWith(
expect.anything(),
'RemoteAgent',
'https://example.com/card',
);
// Also verify that the agent's metadata was updated to use the URL as hash
// Use getDefinition because registerAgent might have been called
expect(registry.getDefinition('RemoteAgent')?.metadata?.hash).toBe(
'https://example.com/card',
);
});
});
describe('registration logic', () => {
+57 -2
View File
@@ -5,7 +5,7 @@
*/
import { Storage } from '../config/storage.js';
import { coreEvents, CoreEvent } from '../utils/events.js';
import { CoreEvent, coreEvents } from '../utils/events.js';
import type { AgentOverride, Config } from '../config/config.js';
import type { AgentDefinition, LocalAgentDefinition } from './types.js';
import { loadAgentsFromDirectory } from './agentLoader.js';
@@ -73,6 +73,23 @@ export class AgentRegistry {
coreEvents.emitAgentsRefreshed();
}
/**
* Acknowledges and registers a previously unacknowledged agent.
*/
async acknowledgeAgent(agent: AgentDefinition): Promise<void> {
const ackService = this.config.getAcknowledgedAgentsService();
const projectRoot = this.config.getProjectRoot();
if (agent.metadata?.hash) {
await ackService.acknowledge(
projectRoot,
agent.name,
agent.metadata.hash,
);
await this.registerAgent(agent);
coreEvents.emitAgentsRefreshed();
}
}
/**
* Disposes of resources and removes event listeners.
*/
@@ -115,8 +132,46 @@ export class AgentRegistry {
`Agent loading error: ${error.message}`,
);
}
const ackService = this.config.getAcknowledgedAgentsService();
const projectRoot = this.config.getProjectRoot();
const unacknowledgedAgents: AgentDefinition[] = [];
const agentsToRegister: AgentDefinition[] = [];
for (const agent of projectAgents.agents) {
// If it's a remote agent, use the agentCardUrl as the hash.
// This allows multiple remote agents in a single file to be tracked independently.
if (agent.kind === 'remote') {
if (!agent.metadata) {
agent.metadata = {};
}
agent.metadata.hash = agent.agentCardUrl;
}
if (!agent.metadata?.hash) {
agentsToRegister.push(agent);
continue;
}
const isAcknowledged = await ackService.isAcknowledged(
projectRoot,
agent.name,
agent.metadata.hash,
);
if (isAcknowledged) {
agentsToRegister.push(agent);
} else {
unacknowledgedAgents.push(agent);
}
}
if (unacknowledgedAgents.length > 0) {
coreEvents.emitAgentsDiscovered(unacknowledgedAgents);
}
await Promise.allSettled(
projectAgents.agents.map((agent) => this.registerAgent(agent)),
agentsToRegister.map((agent) => this.registerAgent(agent)),
);
} else {
coreEvents.emitFeedback(
@@ -0,0 +1,169 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { AgentRegistry } from './registry.js';
import { makeFakeConfig } from '../test-utils/config.js';
import type { AgentDefinition } from './types.js';
import { coreEvents } from '../utils/events.js';
import * as tomlLoader from './agentLoader.js';
import { type Config } from '../config/config.js';
import { AcknowledgedAgentsService } from './acknowledgedAgents.js';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import * as os from 'node:os';
// Mock dependencies
vi.mock('./agentLoader.js', () => ({
loadAgentsFromDirectory: vi.fn(),
}));
const MOCK_AGENT_WITH_HASH: AgentDefinition = {
kind: 'local',
name: 'ProjectAgent',
description: 'Project Agent Desc',
inputConfig: { inputSchema: { type: 'object' } },
modelConfig: {
model: 'test',
generateContentConfig: { thinkingConfig: { includeThoughts: true } },
},
runConfig: { maxTimeMinutes: 1 },
promptConfig: { systemPrompt: 'test' },
metadata: {
hash: 'hash123',
filePath: '/project/agent.md',
},
};
describe('AgentRegistry Acknowledgement', () => {
let registry: AgentRegistry;
let config: Config;
let tempDir: string;
let originalGeminiCliHome: string | undefined;
let ackService: AcknowledgedAgentsService;
beforeEach(async () => {
// Create a unique temp directory for each test
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-cli-test-'));
// Override GEMINI_CLI_HOME to point to the temp directory
originalGeminiCliHome = process.env['GEMINI_CLI_HOME'];
process.env['GEMINI_CLI_HOME'] = tempDir;
ackService = new AcknowledgedAgentsService();
config = makeFakeConfig({
folderTrust: true,
trustedFolder: true,
});
// Ensure we are in trusted folder mode for project agents to load
vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true);
vi.spyOn(config, 'getFolderTrust').mockReturnValue(true);
vi.spyOn(config, 'getProjectRoot').mockReturnValue('/project');
vi.spyOn(config, 'getAcknowledgedAgentsService').mockReturnValue(
ackService,
);
// We cannot easily spy on storage.getProjectAgentsDir if it's a property/getter unless we cast to any or it's a method
// Assuming it's a method on Storage class
vi.spyOn(config.storage, 'getProjectAgentsDir').mockReturnValue(
'/project/.gemini/agents',
);
vi.spyOn(config, 'isAgentsEnabled').mockReturnValue(true);
registry = new AgentRegistry(config);
vi.mocked(tomlLoader.loadAgentsFromDirectory).mockImplementation(
async (dir) => {
if (dir === '/project/.gemini/agents') {
return {
agents: [MOCK_AGENT_WITH_HASH],
errors: [],
};
}
return { agents: [], errors: [] };
},
);
});
afterEach(async () => {
vi.restoreAllMocks();
// Restore environment variable
if (originalGeminiCliHome) {
process.env['GEMINI_CLI_HOME'] = originalGeminiCliHome;
} else {
delete process.env['GEMINI_CLI_HOME'];
}
// Clean up temp directory
await fs.rm(tempDir, { recursive: true, force: true });
});
it('should not register unacknowledged project agents and emit event', async () => {
const emitSpy = vi.spyOn(coreEvents, 'emitAgentsDiscovered');
await registry.initialize();
expect(registry.getDefinition('ProjectAgent')).toBeUndefined();
expect(emitSpy).toHaveBeenCalledWith([MOCK_AGENT_WITH_HASH]);
});
it('should register acknowledged project agents', async () => {
// Acknowledge the agent explicitly
await ackService.acknowledge('/project', 'ProjectAgent', 'hash123');
vi.mocked(tomlLoader.loadAgentsFromDirectory).mockImplementation(
async (dir) => {
if (dir === '/project/.gemini/agents') {
return {
agents: [MOCK_AGENT_WITH_HASH],
errors: [],
};
}
return { agents: [], errors: [] };
},
);
const emitSpy = vi.spyOn(coreEvents, 'emitAgentsDiscovered');
await registry.initialize();
expect(registry.getDefinition('ProjectAgent')).toBeDefined();
expect(emitSpy).not.toHaveBeenCalled();
});
it('should register agents without hash (legacy/safe?)', async () => {
// Current logic: if no hash, allow it.
const agentNoHash = { ...MOCK_AGENT_WITH_HASH, metadata: undefined };
vi.mocked(tomlLoader.loadAgentsFromDirectory).mockImplementation(
async (dir) => {
if (dir === '/project/.gemini/agents') {
return {
agents: [agentNoHash],
errors: [],
};
}
return { agents: [], errors: [] };
},
);
await registry.initialize();
expect(registry.getDefinition('ProjectAgent')).toBeDefined();
});
it('acknowledgeAgent should acknowledge and register agent', async () => {
await registry.acknowledgeAgent(MOCK_AGENT_WITH_HASH);
// Verify against real service state
expect(
await ackService.isAcknowledged('/project', 'ProjectAgent', 'hash123'),
).toBe(true);
expect(registry.getDefinition('ProjectAgent')).toBeDefined();
});
});
+4
View File
@@ -74,6 +74,10 @@ export interface BaseAgentDefinition<
experimental?: boolean;
inputConfig: InputConfig;
outputConfig?: OutputConfig<TOutput>;
metadata?: {
hash?: string;
filePath?: string;
};
}
export interface LocalAgentDefinition<
+7
View File
@@ -100,6 +100,7 @@ import type { FetchAdminControlsResponse } from '../code_assist/types.js';
import { getCodeAssistServer } from '../code_assist/codeAssist.js';
import type { Experiments } from '../code_assist/experiments/experiments.js';
import { AgentRegistry } from '../agents/registry.js';
import { AcknowledgedAgentsService } from '../agents/acknowledgedAgents.js';
import { setGlobalProxy } from '../utils/fetch.js';
import { SubagentTool } from '../agents/subagent-tool.js';
import { getExperiments } from '../code_assist/experiments/experiments.js';
@@ -416,6 +417,7 @@ export class Config {
private promptRegistry!: PromptRegistry;
private resourceRegistry!: ResourceRegistry;
private agentRegistry!: AgentRegistry;
private readonly acknowledgedAgentsService: AcknowledgedAgentsService;
private skillManager!: SkillManager;
private sessionId: string;
private clientVersion: string;
@@ -705,6 +707,7 @@ export class Config {
params.approvalMode ?? params.policyEngineConfig?.approvalMode,
});
this.messageBus = new MessageBus(this.policyEngine, this.debugMode);
this.acknowledgedAgentsService = new AcknowledgedAgentsService();
this.skillManager = new SkillManager();
this.outputSettings = {
format: params.output?.format ?? OutputFormat.TEXT,
@@ -1138,6 +1141,10 @@ export class Config {
return this.agentRegistry;
}
getAcknowledgedAgentsService(): AcknowledgedAgentsService {
return this.acknowledgedAgentsService;
}
getToolRegistry(): ToolRegistry {
return this.toolRegistry;
}
+8
View File
@@ -66,6 +66,14 @@ export class Storage {
return path.join(Storage.getGlobalGeminiDir(), 'agents');
}
static getAcknowledgedAgentsPath(): string {
return path.join(
Storage.getGlobalGeminiDir(),
'acknowledgments',
'agents.json',
);
}
static getSystemSettingsPath(): string {
if (process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']) {
return process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH'];
+18
View File
@@ -5,6 +5,7 @@
*/
import { EventEmitter } from 'node:events';
import type { AgentDefinition } from '../agents/types.js';
import type { McpClient } from '../tools/mcp-client.js';
import type { ExtensionEvents } from './extensionLoader.js';
@@ -110,6 +111,13 @@ export interface RetryAttemptPayload {
model: string;
}
/**
* Payload for the 'agents-discovered' event.
*/
export interface AgentsDiscoveredPayload {
agents: AgentDefinition[];
}
export enum CoreEvent {
UserFeedback = 'user-feedback',
ModelChanged = 'model-changed',
@@ -125,6 +133,7 @@ export enum CoreEvent {
AgentsRefreshed = 'agents-refreshed',
AdminSettingsChanged = 'admin-settings-changed',
RetryAttempt = 'retry-attempt',
AgentsDiscovered = 'agents-discovered',
}
export interface CoreEvents extends ExtensionEvents {
@@ -142,6 +151,7 @@ export interface CoreEvents extends ExtensionEvents {
[CoreEvent.AgentsRefreshed]: never[];
[CoreEvent.AdminSettingsChanged]: never[];
[CoreEvent.RetryAttempt]: [RetryAttemptPayload];
[CoreEvent.AgentsDiscovered]: [AgentsDiscoveredPayload];
}
type EventBacklogItem = {
@@ -264,6 +274,14 @@ export class CoreEventEmitter extends EventEmitter<CoreEvents> {
this.emit(CoreEvent.RetryAttempt, payload);
}
/**
* Notifies subscribers that new unacknowledged agents have been discovered.
*/
emitAgentsDiscovered(agents: AgentDefinition[]): void {
const payload: AgentsDiscoveredPayload = { agents };
this._emitOrQueue(CoreEvent.AgentsDiscovered, payload);
}
/**
* Flushes buffered messages. Call this immediately after primary UI listener
* subscribes.