feat(admin): support admin-enforced settings for Agent Skills (#16406)

This commit is contained in:
N. Taylor Mullen
2026-01-13 23:40:23 -08:00
committed by GitHub
parent 66e7b479ae
commit bb6c574144
20 changed files with 350 additions and 52 deletions
+5
View File
@@ -637,6 +637,7 @@ export async function loadCliConfig(
const mcpEnabled = settings.admin?.mcp?.enabled ?? true;
const extensionsEnabled = settings.admin?.extensions?.enabled ?? true;
const adminSkillsEnabled = settings.admin?.skills?.enabled ?? true;
return new Config({
sessionId,
@@ -661,6 +662,7 @@ export async function loadCliConfig(
mcpEnabled,
extensionsEnabled,
agents: settings.agents,
adminSkillsEnabled,
allowedMcpServers: mcpEnabled
? (argv.allowedMcpServerNames ?? settings.mcp?.allowed)
: undefined,
@@ -708,6 +710,7 @@ export async function loadCliConfig(
enableAgents: settings.experimental?.enableAgents,
skillsSupport: settings.experimental?.skills,
disabledSkills: settings.skills?.disabled,
experimentalJitContext: settings.experimental?.jitContext,
noBrowser: !!process.env['NO_BROWSER'],
summarizeToolOutput: settings.model?.summarizeToolOutput,
@@ -750,6 +753,8 @@ export async function loadCliConfig(
const refreshedSettings = loadSettings(cwd);
return {
disabledSkills: refreshedSettings.merged.skills?.disabled,
adminSkillsEnabled:
refreshedSettings.merged.admin?.skills?.enabled ?? adminSkillsEnabled,
};
},
});
@@ -26,11 +26,12 @@ vi.mock('node:os', async (importOriginal) => {
// Mock @google/gemini-cli-core
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@google/gemini-cli-core')>();
const core = await importOriginal<typeof import('@google/gemini-cli-core')>();
return {
...actual,
...core,
homedir: mockHomedir,
loadAgentsFromDirectory: core.loadAgentsFromDirectory,
loadSkillsFromDir: core.loadSkillsFromDir,
};
});
@@ -10,6 +10,10 @@ import * as path from 'node:path';
import * as os from 'node:os';
import { ExtensionManager } from './extension-manager.js';
import type { Settings } from './settings.js';
import {
loadAgentsFromDirectory,
loadSkillsFromDir,
} from '@google/gemini-cli-core';
let currentTempHome = '';
@@ -24,6 +28,11 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
error: vi.fn(),
warn: vi.fn(),
},
loadAgentsFromDirectory: vi.fn().mockImplementation(async () => ({
agents: [],
errors: [],
})),
loadSkillsFromDir: vi.fn().mockImplementation(async () => []),
};
});
@@ -34,6 +43,11 @@ describe('ExtensionManager Settings Scope', () => {
let extensionDir: string;
beforeEach(async () => {
vi.mocked(loadAgentsFromDirectory).mockResolvedValue({
agents: [],
errors: [],
});
vi.mocked(loadSkillsFromDir).mockResolvedValue([]);
currentTempHome = fs.mkdtempSync(
path.join(os.tmpdir(), 'gemini-cli-test-home-'),
);
@@ -31,6 +31,12 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
return {
...actual,
homedir: mockHomedir,
loadAgentsFromDirectory: vi
.fn()
.mockImplementation(async () => ({ agents: [], errors: [] })),
loadSkillsFromDir: (
await importOriginal<typeof import('@google/gemini-cli-core')>()
).loadSkillsFromDir,
};
});
+11
View File
@@ -23,6 +23,8 @@ import {
ExtensionDisableEvent,
ExtensionEnableEvent,
KeychainTokenStorage,
loadAgentsFromDirectory,
loadSkillsFromDir,
} from '@google/gemini-cli-core';
import { loadSettings, SettingScope } from './settings.js';
import {
@@ -117,6 +119,10 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
listSecrets: vi.fn(),
isAvailable: vi.fn().mockResolvedValue(true),
})),
loadAgentsFromDirectory: vi
.fn()
.mockImplementation(async () => ({ agents: [], errors: [] })),
loadSkillsFromDir: vi.fn().mockImplementation(async () => []),
};
});
@@ -171,6 +177,11 @@ describe('extension tests', () => {
(
KeychainTokenStorage as unknown as ReturnType<typeof vi.fn>
).mockImplementation(() => mockKeychainStorage);
vi.mocked(loadAgentsFromDirectory).mockResolvedValue({
agents: [],
errors: [],
});
vi.mocked(loadSkillsFromDir).mockResolvedValue([]);
tempHomeDir = fs.mkdtempSync(
path.join(os.tmpdir(), 'gemini-cli-test-home-'),
);
+8 -12
View File
@@ -324,23 +324,19 @@ export class LoadedSettings {
setRemoteAdminSettings(remoteSettings: GeminiCodeAssistSetting): void {
const admin: Settings['admin'] = {};
const { secureModeEnabled, mcpSetting, cliFeatureSetting } = remoteSettings;
if (remoteSettings.secureModeEnabled !== undefined) {
admin.secureModeEnabled = remoteSettings.secureModeEnabled;
if (secureModeEnabled !== undefined) {
admin.secureModeEnabled = secureModeEnabled;
}
if (remoteSettings.mcpSetting?.mcpEnabled !== undefined) {
admin.mcp = { enabled: remoteSettings.mcpSetting.mcpEnabled };
if (mcpSetting?.mcpEnabled !== undefined) {
admin.mcp = { enabled: mcpSetting.mcpEnabled };
}
if (
remoteSettings.cliFeatureSetting?.extensionsSetting?.extensionsEnabled !==
undefined
) {
admin.extensions = {
enabled:
remoteSettings.cliFeatureSetting.extensionsSetting.extensionsEnabled,
};
const extensionsSetting = cliFeatureSetting?.extensionsSetting;
if (extensionsSetting?.extensionsEnabled !== undefined) {
admin.extensions = { enabled: extensionsSetting.extensionsEnabled };
}
this._remoteAdminSettings = { admin };
+22
View File
@@ -1842,6 +1842,28 @@ const SETTINGS_SCHEMA = {
},
},
},
skills: {
type: 'object',
label: 'Skills Settings',
category: 'Admin',
requiresRestart: false,
default: {},
description: 'Agent Skills-specific admin settings.',
showInDialog: false,
mergeStrategy: MergeStrategy.REPLACE,
properties: {
enabled: {
type: 'boolean',
label: 'Skills Enabled',
category: 'Admin',
requiresRestart: false,
default: true,
description: 'If false, disallows agent skills from being used.',
showInDialog: false,
mergeStrategy: MergeStrategy.REPLACE,
},
},
},
},
},
} as const satisfies SettingsSchema;
@@ -139,7 +139,26 @@ export class BuiltinCommandLoader implements ICommandLoader {
statsCommand,
themeCommand,
toolsCommand,
...(this.config?.isSkillsSupportEnabled() ? [skillsCommand] : []),
...(this.config?.isSkillsSupportEnabled()
? this.config?.getSkillManager()?.isAdminEnabled() === false
? [
{
name: 'skills',
description: 'Manage agent skills',
kind: CommandKind.BUILT_IN,
autoExecute: false,
subCommands: [],
action: async (
_context: CommandContext,
): Promise<MessageActionReturn> => ({
type: 'message',
messageType: 'error',
content: 'Agent skills are disabled by your admin.',
}),
},
]
: [skillsCommand]
: []),
settingsCommand,
vimCommand,
setupGithubCommand,
@@ -16,6 +16,9 @@ import {
refreshServerHierarchicalMemory,
SimpleExtensionLoader,
type FileDiscoveryService,
showMemory,
addMemory,
listMemoryFiles,
} from '@google/gemini-cli-core';
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
@@ -44,6 +47,9 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
content: 'Memory refreshed successfully.',
};
}),
showMemory: vi.fn(),
addMemory: vi.fn(),
listMemoryFiles: vi.fn(),
refreshServerHierarchicalMemory: vi.fn(),
};
});
@@ -78,6 +84,22 @@ describe('memoryCommand', () => {
mockGetUserMemory = vi.fn();
mockGetGeminiMdFileCount = vi.fn();
vi.mocked(showMemory).mockImplementation((config) => {
const memoryContent = config.getUserMemory() || '';
const fileCount = config.getGeminiMdFileCount() || 0;
let content;
if (memoryContent.length > 0) {
content = `Current memory content from ${fileCount} file(s):\n\n---\n${memoryContent}\n---`;
} else {
content = 'Memory is currently empty.';
}
return {
type: 'message',
messageType: 'info',
content,
};
});
mockContext = createMockCommandContext({
services: {
config: {
@@ -131,6 +153,20 @@ describe('memoryCommand', () => {
beforeEach(() => {
addCommand = getSubCommand('add');
vi.mocked(addMemory).mockImplementation((args) => {
if (!args || args.trim() === '') {
return {
type: 'message',
messageType: 'error',
content: 'Usage: /memory add <text to remember>',
};
}
return {
type: 'tool',
toolName: 'save_memory',
toolArgs: { fact: args.trim() },
};
});
mockContext = createMockCommandContext();
});
@@ -360,6 +396,21 @@ describe('memoryCommand', () => {
beforeEach(() => {
listCommand = getSubCommand('list');
mockGetGeminiMdfilePaths = vi.fn();
vi.mocked(listMemoryFiles).mockImplementation((config) => {
const filePaths = config.getGeminiMdFilePaths() || [];
const fileCount = filePaths.length;
let content;
if (fileCount > 0) {
content = `There are ${fileCount} GEMINI.md file(s) in use:\n\n${filePaths.join('\n')}`;
} else {
content = 'No GEMINI.md files in use.';
}
return {
type: 'message',
messageType: 'info',
content,
};
});
mockContext = createMockCommandContext({
services: {
config: {
@@ -46,6 +46,7 @@ describe('skillsCommand', () => {
getSkillManager: vi.fn().mockReturnValue({
getAllSkills: vi.fn().mockReturnValue(skills),
getSkills: vi.fn().mockReturnValue(skills),
isAdminEnabled: vi.fn().mockReturnValue(true),
getSkill: vi
.fn()
.mockImplementation(
@@ -307,6 +308,43 @@ describe('skillsCommand', () => {
type: MessageType.ERROR,
text: 'Skill "non-existent" not found.',
}),
expect.any(Number),
);
});
it('should show error if skills are disabled by admin during disable', async () => {
const skillManager = context.services.config!.getSkillManager();
vi.mocked(skillManager.isAdminEnabled).mockReturnValue(false);
const disableCmd = skillsCommand.subCommands!.find(
(s) => s.name === 'disable',
)!;
await disableCmd.action!(context, 'skill1');
expect(context.ui.addItem).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageType.ERROR,
text: 'Agent skills are disabled by your admin.',
}),
expect.any(Number),
);
});
it('should show error if skills are disabled by admin during enable', async () => {
const skillManager = context.services.config!.getSkillManager();
vi.mocked(skillManager.isAdminEnabled).mockReturnValue(false);
const enableCmd = skillsCommand.subCommands!.find(
(s) => s.name === 'enable',
)!;
await enableCmd.action!(context, 'skill1');
expect(context.ui.addItem).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageType.ERROR,
text: 'Agent skills are disabled by your admin.',
}),
expect.any(Number),
);
});
});
+30 -4
View File
@@ -79,12 +79,26 @@ async function disableAction(
return;
}
const skillManager = context.services.config?.getSkillManager();
if (skillManager?.isAdminEnabled() === false) {
context.ui.addItem(
{
type: MessageType.ERROR,
text: 'Agent skills are disabled by your admin.',
},
Date.now(),
);
return;
}
const skill = skillManager?.getSkill(skillName);
if (!skill) {
context.ui.addItem({
type: MessageType.ERROR,
text: `Skill "${skillName}" not found.`,
});
context.ui.addItem(
{
type: MessageType.ERROR,
text: `Skill "${skillName}" not found.`,
},
Date.now(),
);
return;
}
@@ -121,6 +135,18 @@ async function enableAction(
return;
}
const skillManager = context.services.config?.getSkillManager();
if (skillManager?.isAdminEnabled() === false) {
context.ui.addItem(
{
type: MessageType.ERROR,
text: 'Agent skills are disabled by your admin.',
},
Date.now(),
);
return;
}
const result = enableSkill(context.services.settings, skillName);
let feedback = renderSkillActionFeedback(
@@ -4,13 +4,17 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { vi } from 'vitest';
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { createExtension } from '../../test-utils/createExtension.js';
import { useExtensionUpdates } from './useExtensionUpdates.js';
import { GEMINI_DIR } from '@google/gemini-cli-core';
import {
GEMINI_DIR,
loadAgentsFromDirectory,
loadSkillsFromDir,
} from '@google/gemini-cli-core';
import { render } from '../../test-utils/render.js';
import { waitFor } from '../../test-utils/async.js';
import { MessageType } from '../types.js';
@@ -36,6 +40,10 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
return {
...actual,
homedir: () => os.homedir(),
loadAgentsFromDirectory: vi
.fn()
.mockResolvedValue({ agents: [], errors: [] }),
loadSkillsFromDir: vi.fn().mockResolvedValue([]),
};
});
@@ -51,6 +59,11 @@ describe('useExtensionUpdates', () => {
let extensionManager: ExtensionManager;
beforeEach(() => {
vi.mocked(loadAgentsFromDirectory).mockResolvedValue({
agents: [],
errors: [],
});
vi.mocked(loadSkillsFromDir).mockResolvedValue([]);
tempHomeDir = fs.mkdtempSync(
path.join(os.tmpdir(), 'gemini-cli-test-home-'),
);
+25
View File
@@ -2091,5 +2091,30 @@ describe('Config JIT Initialization', () => {
expect(skillManager.setDisabledSkills).toHaveBeenCalledWith([]);
});
it('should update admin settings from onReload', async () => {
const mockOnReload = vi.fn().mockResolvedValue({
adminSkillsEnabled: false,
});
const params: ConfigParameters = {
sessionId: 'test-session',
targetDir: '/tmp/test',
debugMode: false,
model: 'test-model',
cwd: '/tmp/test',
skillsSupport: true,
onReload: mockOnReload,
};
config = new Config(params);
await config.initialize();
const skillManager = config.getSkillManager();
vi.spyOn(skillManager, 'setAdminSettings');
await config.reloadSkills();
expect(skillManager.setAdminSettings).toHaveBeenCalledWith(false);
});
});
});
+44 -25
View File
@@ -377,13 +377,17 @@ export interface ConfigParameters {
enableAgents?: boolean;
skillsSupport?: boolean;
disabledSkills?: string[];
adminSkillsEnabled?: boolean;
experimentalJitContext?: boolean;
disableLLMCorrection?: boolean;
onModelChange?: (model: string) => void;
mcpEnabled?: boolean;
extensionsEnabled?: boolean;
agents?: AgentSettings;
onReload?: () => Promise<{ disabledSkills?: string[] }>;
onReload?: () => Promise<{
disabledSkills?: string[];
adminSkillsEnabled?: boolean;
}>;
}
export class Config {
@@ -511,13 +515,17 @@ export class Config {
private hookSystem?: HookSystem;
private readonly onModelChange: ((model: string) => void) | undefined;
private readonly onReload:
| (() => Promise<{ disabledSkills?: string[] }>)
| (() => Promise<{
disabledSkills?: string[];
adminSkillsEnabled?: boolean;
}>)
| undefined;
private readonly enableAgents: boolean;
private readonly agents: AgentSettings;
private readonly skillsSupport: boolean;
private disabledSkills: string[];
private readonly adminSkillsEnabled: boolean;
private readonly experimentalJitContext: boolean;
private readonly disableLLMCorrection: boolean;
@@ -594,6 +602,7 @@ export class Config {
this.disableLLMCorrection = params.disableLLMCorrection ?? false;
this.skillsSupport = params.skillsSupport ?? false;
this.disabledSkills = params.disabledSkills ?? [];
this.adminSkillsEnabled = params.adminSkillsEnabled ?? true;
this.modelAvailabilityService = new ModelAvailabilityService();
this.previewFeatures = params.previewFeatures ?? undefined;
this.experimentalJitContext = params.experimentalJitContext ?? false;
@@ -777,20 +786,22 @@ export class Config {
]);
initMcpHandle?.end();
// Discover skills if enabled
if (this.skillsSupport) {
await this.getSkillManager().discoverSkills(
this.storage,
this.getExtensions(),
);
this.getSkillManager().setDisabledSkills(this.disabledSkills);
// Re-register ActivateSkillTool to update its schema with the discovered enabled skill enums
if (this.getSkillManager().getSkills().length > 0) {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
this.getToolRegistry().registerTool(
new ActivateSkillTool(this, this.messageBus),
this.getSkillManager().setAdminSettings(this.adminSkillsEnabled);
if (this.adminSkillsEnabled) {
await this.getSkillManager().discoverSkills(
this.storage,
this.getExtensions(),
);
this.getSkillManager().setDisabledSkills(this.disabledSkills);
// Re-register ActivateSkillTool to update its schema with the discovered enabled skill enums
if (this.getSkillManager().getSkills().length > 0) {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
this.getToolRegistry().registerTool(
new ActivateSkillTool(this, this.messageBus),
);
}
}
}
@@ -1593,21 +1604,29 @@ export class Config {
if (this.onReload) {
const refreshed = await this.onReload();
this.disabledSkills = refreshed.disabledSkills ?? [];
this.getSkillManager().setAdminSettings(
refreshed.adminSkillsEnabled ?? this.adminSkillsEnabled,
);
}
await this.getSkillManager().discoverSkills(
this.storage,
this.getExtensions(),
);
this.getSkillManager().setDisabledSkills(this.disabledSkills);
// Re-register ActivateSkillTool to update its schema with the newly discovered skills
if (this.getSkillManager().getSkills().length > 0) {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
this.getToolRegistry().registerTool(
new ActivateSkillTool(this, this.messageBus),
if (this.getSkillManager().isAdminEnabled()) {
await this.getSkillManager().discoverSkills(
this.storage,
this.getExtensions(),
);
this.getSkillManager().setDisabledSkills(this.disabledSkills);
// Re-register ActivateSkillTool to update its schema with the newly discovered skills
if (this.getSkillManager().getSkills().length > 0) {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
this.getToolRegistry().registerTool(
new ActivateSkillTool(this, this.messageBus),
);
} else {
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
}
} else {
this.getSkillManager().clearSkills();
this.getToolRegistry().unregisterTool(ActivateSkillTool.Name);
}
+3 -3
View File
@@ -11,8 +11,6 @@ import nodePath from 'node:path';
import type { PolicySettings } from './types.js';
import { ApprovalMode, PolicyDecision, InProcessCheckerType } from './types.js';
import { Storage } from '../config/storage.js';
afterEach(() => {
vi.clearAllMocks();
vi.restoreAllMocks();
@@ -20,7 +18,9 @@ afterEach(() => {
});
describe('createPolicyEngineConfig', () => {
beforeEach(() => {
beforeEach(async () => {
vi.resetModules();
const { Storage } = await import('../config/storage.js');
// Mock Storage to avoid picking up real user/system policies from the host environment
vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(
'/non/existent/user/policies',
+17 -1
View File
@@ -189,7 +189,7 @@ description: project-desc
name: skill1
description: desc1
---
`,
body1`,
);
const storage = new Storage('/dummy');
@@ -247,4 +247,20 @@ description: desc1
expect(enabled).toHaveLength(2);
expect(enabled.map((s) => s.name)).toContain('builtin-skill');
});
it('should maintain admin settings state', async () => {
const service = new SkillManager();
// Case 1: Enabled by admin
service.setAdminSettings(true);
expect(service.isAdminEnabled()).toBe(true);
// Case 2: Disabled by admin
service.setAdminSettings(false);
expect(service.isAdminEnabled()).toBe(false);
});
});
+15
View File
@@ -15,6 +15,7 @@ export { type SkillDefinition };
export class SkillManager {
private skills: SkillDefinition[] = [];
private activeSkillNames: Set<string> = new Set();
private adminSkillsEnabled = true;
/**
* Clears all discovered skills.
@@ -23,6 +24,20 @@ export class SkillManager {
this.skills = [];
}
/**
* Sets administrative settings for skills.
*/
setAdminSettings(enabled: boolean): void {
this.adminSkillsEnabled = enabled;
}
/**
* Returns true if skills are enabled by the admin.
*/
isAdminEnabled(): boolean {
return this.adminSkillsEnabled;
}
/**
* Discovers skills from standard user and project locations, as well as extensions.
* Precedence: Extensions (lowest) -> User -> Project (highest).
@@ -278,7 +278,7 @@ describe('WorkspaceContext with real filesystem', () => {
// handle it gracefully and return false.
expect(workspaceContext.isPathWithinWorkspace(linkA)).toBe(false);
expect(workspaceContext.isPathWithinWorkspace(linkB)).toBe(false);
});
}, 30000);
});
});