feat(a2a): propagate agent settings to experimental A2A server

This commit is contained in:
Alisa Novikova
2026-03-06 07:34:29 -08:00
parent 7f2ab59753
commit 1390a02b10
8 changed files with 309 additions and 106 deletions
+12 -8
View File
@@ -26,7 +26,7 @@ import {
getPersistedState, getPersistedState,
setPersistedState, setPersistedState,
type StateChange, type StateChange,
type AgentSettings, type AgentSettings as CoderAgentSettings,
type PersistedStateMetadata, type PersistedStateMetadata,
getContextIdFromMetadata, getContextIdFromMetadata,
getAgentSettingsFromMetadata, getAgentSettingsFromMetadata,
@@ -44,9 +44,9 @@ import { pushTaskStateFailed } from '../utils/executor_utils.js';
*/ */
class TaskWrapper { class TaskWrapper {
task: Task; task: Task;
agentSettings: AgentSettings; agentSettings: CoderAgentSettings;
constructor(task: Task, agentSettings: AgentSettings) { constructor(task: Task, agentSettings: CoderAgentSettings) {
this.task = task; this.task = task;
this.agentSettings = agentSettings; this.agentSettings = agentSettings;
} }
@@ -89,14 +89,18 @@ export class CoderAgentExecutor implements AgentExecutor {
constructor(private taskStore?: TaskStore) {} constructor(private taskStore?: TaskStore) {}
private async getConfig( private async getConfig(
agentSettings: AgentSettings, agentSettings: CoderAgentSettings,
taskId: string, taskId: string,
): Promise<Config> { ): Promise<Config> {
const workspaceRoot = setTargetDir(agentSettings); const workspaceRoot = setTargetDir(agentSettings);
loadEnvironment(); // Will override any global env with workspace envs loadEnvironment(); // Will override any global env with workspace envs
const settings = loadSettings(workspaceRoot); const loadedSettings = loadSettings(workspaceRoot);
const extensions = loadExtensions(workspaceRoot); const extensions = loadExtensions(workspaceRoot);
return loadConfig(settings, new SimpleExtensionLoader(extensions), taskId); return loadConfig(
loadedSettings,
new SimpleExtensionLoader(extensions),
taskId,
);
} }
/** /**
@@ -138,10 +142,10 @@ export class CoderAgentExecutor implements AgentExecutor {
async createTask( async createTask(
taskId: string, taskId: string,
contextId: string, contextId: string,
agentSettingsInput?: AgentSettings, agentSettingsInput?: CoderAgentSettings,
eventBus?: ExecutionEventBus, eventBus?: ExecutionEventBus,
): Promise<TaskWrapper> { ): Promise<TaskWrapper> {
const agentSettings: AgentSettings = agentSettingsInput || { const agentSettings: CoderAgentSettings = agentSettingsInput || {
kind: CoderAgentEvent.StateAgentSettingsEvent, kind: CoderAgentEvent.StateAgentSettingsEvent,
workspacePath: process.cwd(), workspacePath: process.cwd(),
}; };
+142 -42
View File
@@ -7,7 +7,7 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import * as path from 'node:path'; import * as path from 'node:path';
import { loadConfig } from './config.js'; import { loadConfig } from './config.js';
import type { Settings } from './settings.js'; import type { LoadedSettings } from './settings.js';
import { import {
type ExtensionLoader, type ExtensionLoader,
FileDiscoveryService, FileDiscoveryService,
@@ -40,6 +40,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
}, },
}, },
}), }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}; };
@@ -72,13 +73,35 @@ vi.mock('../utils/logger.js', () => ({
})); }));
describe('loadConfig', () => { describe('loadConfig', () => {
const mockSettings = {} as Settings; const mockLoadedSettings: LoadedSettings = {
userSettings: {},
workspaceSettings: {},
};
const mockExtensionLoader = {} as ExtensionLoader; const mockExtensionLoader = {} as ExtensionLoader;
const taskId = 'test-task-id'; const taskId = 'test-task-id';
beforeEach(() => { beforeEach(() => {
vi.clearAllMocks(); vi.clearAllMocks();
vi.stubEnv('GEMINI_API_KEY', 'test-key'); vi.stubEnv('GEMINI_API_KEY', 'test-key');
// Default mock implementation for Config to include isTrustedFolder
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(Config as any).mockImplementation((params: any) => ({
...params,
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: vi.fn(),
getExperiments: vi.fn().mockReturnValue({
flags: {
[ExperimentFlags.ENABLE_ADMIN_CONTROLS]: {
boolValue: false,
},
},
}),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}));
}); });
afterEach(() => { afterEach(() => {
@@ -87,7 +110,7 @@ describe('loadConfig', () => {
describe('admin settings overrides', () => { describe('admin settings overrides', () => {
it('should not fetch admin controls if experiment is disabled', async () => { it('should not fetch admin controls if experiment is disabled', async () => {
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(fetchAdminControlsOnce).not.toHaveBeenCalled(); expect(fetchAdminControlsOnce).not.toHaveBeenCalled();
}); });
@@ -117,6 +140,7 @@ describe('loadConfig', () => {
}, },
}, },
}), }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn().mockReturnValue({}), getRemoteAdminSettings: vi.fn().mockReturnValue({}),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}; };
@@ -138,7 +162,7 @@ describe('loadConfig', () => {
}; };
vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings);
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenLastCalledWith( expect(Config).toHaveBeenLastCalledWith(
expect.objectContaining({ expect.objectContaining({
@@ -159,7 +183,7 @@ describe('loadConfig', () => {
}; };
vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings);
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenLastCalledWith( expect(Config).toHaveBeenLastCalledWith(
expect.objectContaining({ expect.objectContaining({
@@ -174,7 +198,7 @@ describe('loadConfig', () => {
const mockAdminSettings: FetchAdminControlsResponse = {}; const mockAdminSettings: FetchAdminControlsResponse = {};
vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings);
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenLastCalledWith(expect.objectContaining({})); expect(Config).toHaveBeenLastCalledWith(expect.objectContaining({}));
}); });
@@ -193,7 +217,7 @@ describe('loadConfig', () => {
); );
vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings);
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(fetchAdminControlsOnce).toHaveBeenCalledWith( expect(fetchAdminControlsOnce).toHaveBeenCalledWith(
mockCodeAssistServer, mockCodeAssistServer,
@@ -213,7 +237,11 @@ describe('loadConfig', () => {
it('should set customIgnoreFilePaths when CUSTOM_IGNORE_FILE_PATHS env var is present', async () => { it('should set customIgnoreFilePaths when CUSTOM_IGNORE_FILE_PATHS env var is present', async () => {
const testPath = '/tmp/ignore'; const testPath = '/tmp/ignore';
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath); vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath);
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); const config = await loadConfig(
mockLoadedSettings,
mockExtensionLoader,
taskId,
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([
testPath, testPath,
@@ -222,12 +250,19 @@ describe('loadConfig', () => {
it('should set customIgnoreFilePaths when settings.fileFiltering.customIgnoreFilePaths is present', async () => { it('should set customIgnoreFilePaths when settings.fileFiltering.customIgnoreFilePaths is present', async () => {
const testPath = '/settings/ignore'; const testPath = '/settings/ignore';
const settings: Settings = { const loadedSettings: LoadedSettings = {
fileFiltering: { userSettings: {
customIgnoreFilePaths: [testPath], fileFiltering: {
customIgnoreFilePaths: [testPath],
},
}, },
workspaceSettings: {},
}; };
const config = await loadConfig(settings, mockExtensionLoader, taskId); const config = await loadConfig(
loadedSettings,
mockExtensionLoader,
taskId,
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([
testPath, testPath,
@@ -238,12 +273,19 @@ describe('loadConfig', () => {
const envPath = '/env/ignore'; const envPath = '/env/ignore';
const settingsPath = '/settings/ignore'; const settingsPath = '/settings/ignore';
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', envPath); vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', envPath);
const settings: Settings = { const loadedSettings: LoadedSettings = {
fileFiltering: { userSettings: {
customIgnoreFilePaths: [settingsPath], fileFiltering: {
customIgnoreFilePaths: [settingsPath],
},
}, },
workspaceSettings: {},
}; };
const config = await loadConfig(settings, mockExtensionLoader, taskId); const config = await loadConfig(
loadedSettings,
mockExtensionLoader,
taskId,
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([
settingsPath, settingsPath,
@@ -254,13 +296,21 @@ describe('loadConfig', () => {
it('should split CUSTOM_IGNORE_FILE_PATHS using system delimiter', async () => { it('should split CUSTOM_IGNORE_FILE_PATHS using system delimiter', async () => {
const paths = ['/path/one', '/path/two']; const paths = ['/path/one', '/path/two'];
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', paths.join(path.delimiter)); vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', paths.join(path.delimiter));
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); const config = await loadConfig(
mockLoadedSettings,
mockExtensionLoader,
taskId,
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual(paths); expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual(paths);
}); });
it('should have empty customIgnoreFilePaths when both are missing', async () => { it('should have empty customIgnoreFilePaths when both are missing', async () => {
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); const config = await loadConfig(
mockLoadedSettings,
mockExtensionLoader,
taskId,
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([]); expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([]);
}); });
@@ -268,13 +318,16 @@ describe('loadConfig', () => {
it('should initialize FileDiscoveryService with correct options', async () => { it('should initialize FileDiscoveryService with correct options', async () => {
const testPath = '/tmp/ignore'; const testPath = '/tmp/ignore';
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath); vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath);
const settings: Settings = { const loadedSettings: LoadedSettings = {
fileFiltering: { userSettings: {
respectGitIgnore: false, fileFiltering: {
respectGitIgnore: false,
},
}, },
workspaceSettings: {},
}; };
await loadConfig(settings, mockExtensionLoader, taskId); await loadConfig(loadedSettings, mockExtensionLoader, taskId);
expect(FileDiscoveryService).toHaveBeenCalledWith(expect.any(String), { expect(FileDiscoveryService).toHaveBeenCalledWith(expect.any(String), {
respectGitIgnore: false, respectGitIgnore: false,
@@ -285,10 +338,13 @@ describe('loadConfig', () => {
describe('tool configuration', () => { describe('tool configuration', () => {
it('should pass V1 allowedTools to Config properly', async () => { it('should pass V1 allowedTools to Config properly', async () => {
const settings: Settings = { const loadedSettings: LoadedSettings = {
allowedTools: ['shell', 'edit'], userSettings: {
allowedTools: ['shell', 'edit'],
},
workspaceSettings: {},
}; };
await loadConfig(settings, mockExtensionLoader, taskId); await loadConfig(loadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith( expect(Config).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
allowedTools: ['shell', 'edit'], allowedTools: ['shell', 'edit'],
@@ -297,12 +353,15 @@ describe('loadConfig', () => {
}); });
it('should pass V2 tools.allowed to Config properly', async () => { it('should pass V2 tools.allowed to Config properly', async () => {
const settings: Settings = { const loadedSettings: LoadedSettings = {
tools: { userSettings: {
allowed: ['shell', 'fetch'], tools: {
allowed: ['shell', 'fetch'],
},
}, },
workspaceSettings: {},
}; };
await loadConfig(settings, mockExtensionLoader, taskId); await loadConfig(loadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith( expect(Config).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
allowedTools: ['shell', 'fetch'], allowedTools: ['shell', 'fetch'],
@@ -311,13 +370,16 @@ describe('loadConfig', () => {
}); });
it('should prefer V1 allowedTools over V2 tools.allowed if both present', async () => { it('should prefer V1 allowedTools over V2 tools.allowed if both present', async () => {
const settings: Settings = { const loadedSettings: LoadedSettings = {
allowedTools: ['v1-tool'], userSettings: {
tools: { allowedTools: ['v1-tool'],
allowed: ['v2-tool'], tools: {
allowed: ['v2-tool'],
},
}, },
workspaceSettings: {},
}; };
await loadConfig(settings, mockExtensionLoader, taskId); await loadConfig(loadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith( expect(Config).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
allowedTools: ['v1-tool'], allowedTools: ['v1-tool'],
@@ -325,10 +387,42 @@ describe('loadConfig', () => {
); );
}); });
it('should pass agent settings to Config', async () => {
const loadedSettings: LoadedSettings = {
userSettings: {
experimental: {
enableAgents: false,
},
agents: {
overrides: {
test_agent: { enabled: true },
},
},
},
workspaceSettings: {},
};
await loadConfig(loadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith(
expect.objectContaining({
enableAgents: false,
agents: loadedSettings.userSettings.agents,
}),
);
});
it('should default enableAgents to false if not specified (secure default)', async () => {
await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith(
expect.objectContaining({
enableAgents: false,
}),
);
});
describe('interactivity', () => { describe('interactivity', () => {
it('should set interactive true when not headless', async () => { it('should set interactive true when not headless', async () => {
vi.mocked(isHeadlessMode).mockReturnValue(false); vi.mocked(isHeadlessMode).mockReturnValue(false);
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith( expect(Config).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
interactive: true, interactive: true,
@@ -339,7 +433,7 @@ describe('loadConfig', () => {
it('should set interactive false when headless', async () => { it('should set interactive false when headless', async () => {
vi.mocked(isHeadlessMode).mockReturnValue(true); vi.mocked(isHeadlessMode).mockReturnValue(true);
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith( expect(Config).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
interactive: false, interactive: false,
@@ -378,12 +472,13 @@ describe('loadConfig', () => {
waitForMcpInit: vi.fn(), waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock, refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }), getExperiments: vi.fn().mockReturnValue({ flags: {} }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}) as unknown as Config, }) as unknown as Config,
); );
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(refreshAuthMock).toHaveBeenCalledWith( expect(refreshAuthMock).toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE, AuthType.LOGIN_WITH_GOOGLE,
@@ -408,13 +503,14 @@ describe('loadConfig', () => {
waitForMcpInit: vi.fn(), waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock, refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }), getExperiments: vi.fn().mockReturnValue({ flags: {} }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}) as unknown as Config, }) as unknown as Config,
); );
await expect( await expect(
loadConfig(mockSettings, mockExtensionLoader, taskId), loadConfig(mockLoadedSettings, mockExtensionLoader, taskId),
).rejects.toThrow('Non-interactive session'); ).rejects.toThrow('Non-interactive session');
expect(refreshAuthMock).toHaveBeenCalledWith( expect(refreshAuthMock).toHaveBeenCalledWith(
@@ -437,12 +533,13 @@ describe('loadConfig', () => {
waitForMcpInit: vi.fn(), waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock, refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }), getExperiments: vi.fn().mockReturnValue({ flags: {} }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}) as unknown as Config, }) as unknown as Config,
); );
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(refreshAuthMock).not.toHaveBeenCalledWith( expect(refreshAuthMock).not.toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE, AuthType.LOGIN_WITH_GOOGLE,
@@ -464,12 +561,13 @@ describe('loadConfig', () => {
waitForMcpInit: vi.fn(), waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock, refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }), getExperiments: vi.fn().mockReturnValue({ flags: {} }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}) as unknown as Config, }) as unknown as Config,
); );
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId);
expect(refreshAuthMock).not.toHaveBeenCalledWith( expect(refreshAuthMock).not.toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE, AuthType.LOGIN_WITH_GOOGLE,
@@ -490,13 +588,14 @@ describe('loadConfig', () => {
waitForMcpInit: vi.fn(), waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock, refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }), getExperiments: vi.fn().mockReturnValue({ flags: {} }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}) as unknown as Config, }) as unknown as Config,
); );
await expect( await expect(
loadConfig(mockSettings, mockExtensionLoader, taskId), loadConfig(mockLoadedSettings, mockExtensionLoader, taskId),
).rejects.toThrow( ).rejects.toThrow(
'Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.', 'Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.',
); );
@@ -526,13 +625,14 @@ describe('loadConfig', () => {
waitForMcpInit: vi.fn(), waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock, refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }), getExperiments: vi.fn().mockReturnValue({ flags: {} }),
isTrustedFolder: vi.fn().mockReturnValue(true),
getRemoteAdminSettings: vi.fn(), getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(),
}) as unknown as Config, }) as unknown as Config,
); );
await expect( await expect(
loadConfig(mockSettings, mockExtensionLoader, taskId), loadConfig(mockLoadedSettings, mockExtensionLoader, taskId),
).rejects.toThrow( ).rejects.toThrow(
'OAuth failed. Fallback to COMPUTE_ADC also failed: ADC failed', 'OAuth failed. Fallback to COMPUTE_ADC also failed: ADC failed',
); );
+43 -12
View File
@@ -32,17 +32,49 @@ import {
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import { logger } from '../utils/logger.js'; import { logger } from '../utils/logger.js';
import type { Settings } from './settings.js'; import {
import { type AgentSettings, CoderAgentEvent } from '../types.js'; type Settings,
type LoadedSettings,
resolveEnvVarsInObject,
} from './settings.js';
import {
type AgentSettings as CoderAgentSettings,
CoderAgentEvent,
} from '../types.js';
export async function loadConfig( export async function loadConfig(
settings: Settings, loadedSettings: LoadedSettings,
extensionLoader: ExtensionLoader, extensionLoader: ExtensionLoader,
taskId: string, taskId: string,
): Promise<Config> { ): Promise<Config> {
const workspaceDir = process.cwd(); const workspaceDir = process.cwd();
const adcFilePath = process.env['GOOGLE_APPLICATION_CREDENTIALS']; const adcFilePath = process.env['GOOGLE_APPLICATION_CREDENTIALS'];
const { userSettings, workspaceSettings } = loadedSettings;
// Set an initial config to use to determine trust and get a code assist server.
// This is needed to fetch admin controls and safely resolve environment variables.
const initialConfigParams: ConfigParameters = {
sessionId: taskId,
targetDir: workspaceDir,
cwd: workspaceDir,
extensionLoader,
debugMode: process.env['DEBUG'] === 'true' || false,
model: PREVIEW_GEMINI_MODEL,
};
const tempConfig = new Config(initialConfigParams);
// Securely merge settings: only expand workspace variables if the user trusts this folder.
// If there are overlapping keys, the values of workspaceSettings will
// override values from userSettings.
const isTrusted = tempConfig.isTrustedFolder();
const settings: Settings = {
...userSettings,
...(isTrusted
? resolveEnvVarsInObject(workspaceSettings)
: workspaceSettings),
};
const folderTrust = const folderTrust =
settings.folderTrust === true || settings.folderTrust === true ||
process.env['GEMINI_FOLDER_TRUST'] === 'true'; process.env['GEMINI_FOLDER_TRUST'] === 'true';
@@ -110,6 +142,8 @@ export async function loadConfig(
interactive: !isHeadlessMode(), interactive: !isHeadlessMode(),
enableInteractiveShell: !isHeadlessMode(), enableInteractiveShell: !isHeadlessMode(),
ptyInfo: 'auto', ptyInfo: 'auto',
enableAgents: settings.experimental?.enableAgents ?? false,
agents: settings.agents,
}; };
const fileService = new FileDiscoveryService(workspaceDir, { const fileService = new FileDiscoveryService(workspaceDir, {
@@ -129,16 +163,11 @@ export async function loadConfig(
configParams.geminiMdFileCount = fileCount; configParams.geminiMdFileCount = fileCount;
configParams.geminiMdFilePaths = filePaths; configParams.geminiMdFilePaths = filePaths;
// Set an initial config to use to get a code assist server. // Use initialConfig to fetch admin controls.
// This is needed to fetch admin controls. const codeAssistServer = getCodeAssistServer(tempConfig);
const initialConfig = new Config({
...configParams,
});
const codeAssistServer = getCodeAssistServer(initialConfig);
const adminControlsEnabled = const adminControlsEnabled =
initialConfig.getExperiments()?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS] tempConfig.getExperiments()?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS]
?.boolValue ?? false; ?.boolValue ?? false;
// Initialize final config parameters to the previous parameters. // Initialize final config parameters to the previous parameters.
@@ -178,7 +207,9 @@ export async function loadConfig(
return config; return config;
} }
export function setTargetDir(agentSettings: AgentSettings | undefined): string { export function setTargetDir(
agentSettings: CoderAgentSettings | undefined,
): string {
const originalCWD = process.cwd(); const originalCWD = process.cwd();
const targetDir = const targetDir =
process.env['CODER_AGENT_WORKSPACE_PATH'] ?? process.env['CODER_AGENT_WORKSPACE_PATH'] ??
+30 -18
View File
@@ -106,27 +106,41 @@ describe('loadSettings', () => {
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(settings)); fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(settings));
const result = loadSettings(mockWorkspaceDir); const result = loadSettings(mockWorkspaceDir);
expect(result.showMemoryUsage).toBe(true); expect(result.userSettings.showMemoryUsage).toBe(true);
expect(result.coreTools).toEqual(['tool1', 'tool2']); expect(result.userSettings.coreTools).toEqual(['tool1', 'tool2']);
expect(result.mcpServers).toHaveProperty('server1'); expect(result.userSettings.mcpServers).toHaveProperty('server1');
expect(result.fileFiltering?.respectGitIgnore).toBe(true); expect(result.userSettings.fileFiltering?.respectGitIgnore).toBe(true);
}); });
it('should overwrite top-level settings from workspace (shallow merge)', () => { it('should load experimental and agents settings correctly', () => {
const settings = {
experimental: {
enableAgents: true,
},
agents: {
overrides: {
test_agent: { enabled: false },
},
},
};
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(settings));
const result = loadSettings(mockWorkspaceDir);
expect(result.userSettings.experimental?.enableAgents).toBe(true);
expect(result.userSettings.agents?.overrides?.['test_agent']?.enabled).toBe(
false,
);
});
it('should return separate user and raw workspace settings', () => {
const userSettings = { const userSettings = {
showMemoryUsage: false, showMemoryUsage: false,
fileFiltering: {
respectGitIgnore: true,
enableRecursiveFileSearch: true,
},
}; };
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings)); fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
const workspaceSettings = { const workspaceSettings = {
showMemoryUsage: true, showMemoryUsage: true,
fileFiltering: { coreTools: ['${VAR}'],
respectGitIgnore: false,
},
}; };
const workspaceSettingsPath = path.join( const workspaceSettingsPath = path.join(
mockGeminiWorkspaceDir, mockGeminiWorkspaceDir,
@@ -135,11 +149,9 @@ describe('loadSettings', () => {
fs.writeFileSync(workspaceSettingsPath, JSON.stringify(workspaceSettings)); fs.writeFileSync(workspaceSettingsPath, JSON.stringify(workspaceSettings));
const result = loadSettings(mockWorkspaceDir); const result = loadSettings(mockWorkspaceDir);
// Primitive value overwritten expect(result.userSettings.showMemoryUsage).toBe(false);
expect(result.showMemoryUsage).toBe(true); expect(result.workspaceSettings.showMemoryUsage).toBe(true);
// Workspace settings must be RAW (no expansion)
// Object value completely replaced (shallow merge behavior) expect(result.workspaceSettings.coreTools).toEqual(['${VAR}']);
expect(result.fileFiltering?.respectGitIgnore).toBe(false);
expect(result.fileFiltering?.enableRecursiveFileSearch).toBeUndefined();
}); });
}); });
+25 -16
View File
@@ -14,6 +14,7 @@ import {
getErrorMessage, getErrorMessage,
type TelemetrySettings, type TelemetrySettings,
homedir, homedir,
type AgentSettings,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import stripJsonComments from 'strip-json-comments'; import stripJsonComments from 'strip-json-comments';
@@ -48,6 +49,11 @@ export interface Settings {
enableRecursiveFileSearch?: boolean; enableRecursiveFileSearch?: boolean;
customIgnoreFilePaths?: string[]; customIgnoreFilePaths?: string[];
}; };
experimental?: {
enableEventDrivenScheduler?: boolean;
enableAgents?: boolean;
};
agents?: AgentSettings;
} }
export interface SettingsError { export interface SettingsError {
@@ -60,19 +66,25 @@ export interface CheckpointingSettings {
} }
/** /**
* Loads settings from user and workspace directories. * The result of loading settings, containing both user-level (expanded)
* Project settings override user settings. * and workspace-level (raw) configurations.
*
* How is it different to gemini-cli/cli: Returns already merged settings rather
* than `LoadedSettings` (unnecessary since we are not modifying users
* settings.json).
*/ */
export function loadSettings(workspaceDir: string): Settings { export interface LoadedSettings {
userSettings: Settings;
workspaceSettings: Settings;
}
/**
* Loads settings from user and workspace directories.
* Workspace settings are returned RAW (without env var expansion) to prevent
* secret leakage from untrusted repositories.
*/
export function loadSettings(workspaceDir: string): LoadedSettings {
let userSettings: Settings = {}; let userSettings: Settings = {};
let workspaceSettings: Settings = {}; let workspaceSettings: Settings = {};
const settingsErrors: SettingsError[] = []; const settingsErrors: SettingsError[] = [];
// Load user settings // Load user settings (Safe to expand)
try { try {
if (fs.existsSync(USER_SETTINGS_PATH)) { if (fs.existsSync(USER_SETTINGS_PATH)) {
const userContent = fs.readFileSync(USER_SETTINGS_PATH, 'utf-8'); const userContent = fs.readFileSync(USER_SETTINGS_PATH, 'utf-8');
@@ -95,15 +107,14 @@ export function loadSettings(workspaceDir: string): Settings {
'settings.json', 'settings.json',
); );
// Load workspace settings // Load workspace settings (RAW - NO EXPANSION)
try { try {
if (fs.existsSync(workspaceSettingsPath)) { if (fs.existsSync(workspaceSettingsPath)) {
const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8'); const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8');
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const parsedWorkspaceSettings = JSON.parse( workspaceSettings = JSON.parse(
stripJsonComments(projectContent), stripJsonComments(projectContent),
) as Settings; ) as Settings;
workspaceSettings = resolveEnvVarsInObject(parsedWorkspaceSettings);
} }
} catch (error: unknown) { } catch (error: unknown) {
settingsErrors.push({ settingsErrors.push({
@@ -120,11 +131,9 @@ export function loadSettings(workspaceDir: string): Settings {
} }
} }
// If there are overlapping keys, the values of workspaceSettings will
// override values from userSettings
return { return {
...userSettings, userSettings,
...workspaceSettings, workspaceSettings,
}; };
} }
@@ -140,7 +149,7 @@ function resolveEnvVarsInString(value: string): string {
}); });
} }
function resolveEnvVarsInObject<T>(obj: T): T { export function resolveEnvVarsInObject<T>(obj: T): T {
if ( if (
obj === null || obj === null ||
obj === undefined || obj === undefined ||
+4 -4
View File
@@ -18,7 +18,7 @@ import {
import { A2AExpressApp, type UserBuilder } from '@a2a-js/sdk/server/express'; // Import server components import { A2AExpressApp, type UserBuilder } from '@a2a-js/sdk/server/express'; // Import server components
import { v4 as uuidv4 } from 'uuid'; import { v4 as uuidv4 } from 'uuid';
import { logger } from '../utils/logger.js'; import { logger } from '../utils/logger.js';
import type { AgentSettings } from '../types.js'; import { type AgentSettings as CoderAgentSettings } from '../types.js';
import { GCSTaskStore, NoOpTaskStore } from '../persistence/gcs.js'; import { GCSTaskStore, NoOpTaskStore } from '../persistence/gcs.js';
import { CoderAgentExecutor } from '../agent/executor.js'; import { CoderAgentExecutor } from '../agent/executor.js';
import { requestStorage } from './requestStorage.js'; import { requestStorage } from './requestStorage.js';
@@ -197,10 +197,10 @@ export async function createApp() {
// Load the server configuration once on startup. // Load the server configuration once on startup.
const workspaceRoot = setTargetDir(undefined); const workspaceRoot = setTargetDir(undefined);
loadEnvironment(); loadEnvironment();
const settings = loadSettings(workspaceRoot); const loadedSettings = loadSettings(workspaceRoot);
const extensions = loadExtensions(workspaceRoot); const extensions = loadExtensions(workspaceRoot);
const config = await loadConfig( const config = await loadConfig(
settings, loadedSettings,
new SimpleExtensionLoader(extensions), new SimpleExtensionLoader(extensions),
'a2a-server', 'a2a-server',
); );
@@ -252,7 +252,7 @@ export async function createApp() {
const taskId = uuidv4(); const taskId = uuidv4();
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const agentSettings = req.body.agentSettings as const agentSettings = req.body.agentSettings as
| AgentSettings | CoderAgentSettings
| undefined; | undefined;
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const contextId = req.body.contextId || uuidv4(); const contextId = req.body.contextId || uuidv4();
+32 -1
View File
@@ -29,7 +29,7 @@ import { SimpleExtensionLoader } from '../utils/extensionLoader.js';
import type { ToolRegistry } from '../tools/tool-registry.js'; import type { ToolRegistry } from '../tools/tool-registry.js';
import { ThinkingLevel } from '@google/genai'; import { ThinkingLevel } from '@google/genai';
import type { AcknowledgedAgentsService } from './acknowledgedAgents.js'; import type { AcknowledgedAgentsService } from './acknowledgedAgents.js';
import { PolicyDecision } from '../policy/types.js'; import { PolicyDecision, ApprovalMode } from '../policy/types.js';
import { A2AAuthProviderFactory } from './auth-provider/factory.js'; import { A2AAuthProviderFactory } from './auth-provider/factory.js';
import type { A2AAuthProvider } from './auth-provider/types.js'; import type { A2AAuthProvider } from './auth-provider/types.js';
@@ -1171,6 +1171,37 @@ describe('AgentRegistry', () => {
}), }),
); );
}); });
it('should register remote agents with ALLOW decision in YOLO mode', async () => {
const remoteAgent: AgentDefinition = {
kind: 'remote',
name: 'YoloAgent',
description: 'A remote agent in YOLO mode',
agentCardUrl: 'https://example.com/card',
inputConfig: { inputSchema: { type: 'object' } },
};
vi.mocked(A2AClientManager.getInstance).mockReturnValue({
loadAgent: vi.fn().mockResolvedValue({ name: 'YoloAgent' }),
} as unknown as A2AClientManager);
const policyEngine = mockConfig.getPolicyEngine();
vi.spyOn(mockConfig, 'getApprovalMode').mockReturnValue(
ApprovalMode.YOLO,
);
const addRuleSpy = vi.spyOn(policyEngine, 'addRule');
await registry.testRegisterAgent(remoteAgent);
// In YOLO mode, even remote agents should be registered with ALLOW.
expect(addRuleSpy).toHaveBeenLastCalledWith(
expect.objectContaining({
toolName: 'YoloAgent',
decision: PolicyDecision.ALLOW,
source: 'AgentRegistry (Dynamic)',
}),
);
});
}); });
describe('reload', () => { describe('reload', () => {
+21 -5
View File
@@ -23,7 +23,11 @@ import {
type ModelConfig, type ModelConfig,
ModelConfigService, ModelConfigService,
} from '../services/modelConfigService.js'; } from '../services/modelConfigService.js';
import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../policy/types.js'; import {
PolicyDecision,
PRIORITY_SUBAGENT_TOOL,
ApprovalMode,
} from '../policy/types.js';
import { A2AAgentError, AgentAuthConfigMissingError } from './a2a-errors.js'; import { A2AAgentError, AgentAuthConfigMissingError } from './a2a-errors.js';
/** /**
@@ -176,9 +180,8 @@ export class AgentRegistry {
agent.metadata.hash, agent.metadata.hash,
); );
if (isAcknowledged) { agentsToRegister.push(agent);
agentsToRegister.push(agent); if (!isAcknowledged) {
} else {
unacknowledgedAgents.push(agent); unacknowledgedAgents.push(agent);
} }
} }
@@ -340,10 +343,23 @@ export class AgentRegistry {
policyEngine.removeRulesForTool(definition.name, 'AgentRegistry (Dynamic)'); policyEngine.removeRulesForTool(definition.name, 'AgentRegistry (Dynamic)');
// Add the new dynamic policy // Add the new dynamic policy
const isYolo = this.config.getApprovalMode() === ApprovalMode.YOLO;
const isAcknowledged =
definition.kind === 'local' &&
(!definition.metadata?.hash ||
(this.config.getProjectRoot() &&
this.config
.getAcknowledgedAgentsService()
?.isAcknowledgedSync?.(
this.config.getProjectRoot(),
definition.name,
definition.metadata.hash,
)));
policyEngine.addRule({ policyEngine.addRule({
toolName: definition.name, toolName: definition.name,
decision: decision:
definition.kind === 'local' isAcknowledged || isYolo
? PolicyDecision.ALLOW ? PolicyDecision.ALLOW
: PolicyDecision.ASK_USER, : PolicyDecision.ASK_USER,
priority: PRIORITY_SUBAGENT_TOOL, priority: PRIORITY_SUBAGENT_TOOL,