From 272570cc188156ed96fe6fa338656adbc092bbc4 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Fri, 16 Jan 2026 15:10:55 -0800 Subject: [PATCH] feat(agent): enable agent skills by default (#16736) --- packages/cli/src/config/config.ts | 10 +- .../cli/src/config/settingsSchema.test.ts | 11 ++ packages/cli/src/config/settingsSchema.ts | 11 ++ .../skills-backward-compatibility.test.ts | 158 ++++++++++++++++++ .../cli/src/ui/commands/skillsCommand.test.ts | 24 ++- schemas/settings.schema.json | 7 + scripts/generate-settings-doc.ts | 4 + 7 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/config/skills-backward-compatibility.test.ts diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 591d861d7c..c5c61ce748 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -285,16 +285,15 @@ export async function parseArguments( return true; }); - if (settings.experimental.extensionManagement) { + if (settings.experimental?.extensionManagement) { yargsInstance.command(extensionsCommand); } - if (settings.experimental.skills) { + if (settings.experimental?.skills || (settings.skills?.enabled ?? true)) { yargsInstance.command(skillsCommand); } - // Register hooks command if hooks are enabled - if (settings.tools.enableHooks) { + if (settings.tools?.enableHooks) { yargsInstance.command(hooksCommand); } @@ -722,7 +721,8 @@ export async function loadCliConfig( enableExtensionReloading: settings.experimental?.extensionReloading, enableAgents: settings.experimental?.enableAgents, plan: settings.experimental?.plan, - skillsSupport: settings.experimental?.skills, + skillsSupport: + settings.experimental?.skills || (settings.skills?.enabled ?? true), disabledSkills: settings.skills?.disabled, experimentalJitContext: settings.experimental?.jitContext, noBrowser: !!process.env['NO_BROWSER'], diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 1daba61176..f399dfdd1b 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -357,6 +357,17 @@ describe('SettingsSchema', () => { ); }); + it('should have skills setting enabled by default', () => { + const setting = getSettingsSchema().skills.properties.enabled; + expect(setting).toBeDefined(); + expect(setting.type).toBe('boolean'); + expect(setting.category).toBe('Advanced'); + expect(setting.default).toBe(true); + expect(setting.requiresRestart).toBe(true); + expect(setting.showInDialog).toBe(true); + expect(setting.description).toBe('Enable Agent Skills.'); + }); + it('should have plan setting in schema', () => { const setting = getSettingsSchema().experimental.properties.plan; expect(setting).toBeDefined(); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index ba850051a9..9bee1977ec 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -109,6 +109,7 @@ export interface SettingDefinition { key?: string; properties?: SettingsSchema; showInDialog?: boolean; + ignoreInDocs?: boolean; mergeStrategy?: MergeStrategy; /** Enum type options */ options?: readonly SettingEnumOption[]; @@ -1597,6 +1598,16 @@ const SETTINGS_SCHEMA = { description: 'Settings for agent skills.', showInDialog: false, properties: { + enabled: { + type: 'boolean', + label: 'Enable Agent Skills', + category: 'Advanced', + requiresRestart: true, + default: true, + description: 'Enable Agent Skills.', + showInDialog: true, + ignoreInDocs: true, + }, disabled: { type: 'array', label: 'Disabled Skills', diff --git a/packages/cli/src/config/skills-backward-compatibility.test.ts b/packages/cli/src/config/skills-backward-compatibility.test.ts new file mode 100644 index 0000000000..57a2d1e4d0 --- /dev/null +++ b/packages/cli/src/config/skills-backward-compatibility.test.ts @@ -0,0 +1,158 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { loadCliConfig, parseArguments } from './config.js'; +import * as trustedFolders from './trustedFolders.js'; +import { loadServerHierarchicalMemory } from '@google/gemini-cli-core'; +import { type Settings, createTestMergedSettings } from './settings.js'; + +vi.mock('./trustedFolders.js'); +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + loadServerHierarchicalMemory: vi.fn(), + getPty: vi.fn().mockResolvedValue({ name: 'test-pty' }), + getVersion: vi.fn().mockResolvedValue('0.0.0-test'), + }; +}); + +describe('Agent Skills Backward Compatibility', () => { + const originalArgv = process.argv; + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(trustedFolders.isWorkspaceTrusted).mockReturnValue({ + isTrusted: true, + } as unknown as trustedFolders.TrustResult); + }); + + afterEach(() => { + process.argv = originalArgv; + }); + + describe('loadCliConfig', () => { + it('should default skillsSupport to true when no settings are present', async () => { + vi.mocked(loadServerHierarchicalMemory).mockResolvedValue({ + memoryContent: '', + fileCount: 0, + filePaths: [], + }); + + process.argv = ['node', 'gemini']; + const settings = createTestMergedSettings({}); + const config = await loadCliConfig( + settings, + 'test-session', + await parseArguments(settings), + ); + expect( + ( + config as unknown as { isSkillsSupportEnabled: () => boolean } + ).isSkillsSupportEnabled(), + ).toBe(true); + }); + + it('should prioritize skills.enabled=false from settings', async () => { + vi.mocked(loadServerHierarchicalMemory).mockResolvedValue({ + memoryContent: '', + fileCount: 0, + filePaths: [], + }); + + const settings = createTestMergedSettings({ + skills: { enabled: false }, + } as unknown as Settings); + + process.argv = ['node', 'gemini']; + const config = await loadCliConfig( + settings, + 'test-session', + await parseArguments(settings), + ); + expect( + ( + config as unknown as { isSkillsSupportEnabled: () => boolean } + ).isSkillsSupportEnabled(), + ).toBe(false); + }); + + it('should support legacy experimental.skills=true from settings', async () => { + vi.mocked(loadServerHierarchicalMemory).mockResolvedValue({ + memoryContent: '', + fileCount: 0, + filePaths: [], + }); + + const settings = createTestMergedSettings({ + experimental: { skills: true }, + } as unknown as Settings); + + process.argv = ['node', 'gemini']; + const config = await loadCliConfig( + settings, + 'test-session', + await parseArguments(settings), + ); + expect( + ( + config as unknown as { isSkillsSupportEnabled: () => boolean } + ).isSkillsSupportEnabled(), + ).toBe(true); + }); + + it('should prioritize legacy experimental.skills=true over new skills.enabled=false', async () => { + vi.mocked(loadServerHierarchicalMemory).mockResolvedValue({ + memoryContent: '', + fileCount: 0, + filePaths: [], + }); + + const settings = createTestMergedSettings({ + skills: { enabled: false }, + experimental: { skills: true }, + } as unknown as Settings); + + process.argv = ['node', 'gemini']; + const config = await loadCliConfig( + settings, + 'test-session', + await parseArguments(settings), + ); + expect( + ( + config as unknown as { isSkillsSupportEnabled: () => boolean } + ).isSkillsSupportEnabled(), + ).toBe(true); + }); + + it('should still be enabled by default if legacy experimental.skills is false (since new default is true)', async () => { + vi.mocked(loadServerHierarchicalMemory).mockResolvedValue({ + memoryContent: '', + fileCount: 0, + filePaths: [], + }); + + const settings = createTestMergedSettings({ + experimental: { skills: false }, + } as unknown as Settings); + + process.argv = ['node', 'gemini']; + const config = await loadCliConfig( + settings, + 'test-session', + await parseArguments(settings), + ); + expect( + ( + config as unknown as { isSkillsSupportEnabled: () => boolean } + ).isSkillsSupportEnabled(), + ).toBe(true); + }); + }); +}); diff --git a/packages/cli/src/ui/commands/skillsCommand.test.ts b/packages/cli/src/ui/commands/skillsCommand.test.ts index 8d55d343e6..7331f31330 100644 --- a/packages/cli/src/ui/commands/skillsCommand.test.ts +++ b/packages/cli/src/ui/commands/skillsCommand.test.ts @@ -10,7 +10,12 @@ import { MessageType, type HistoryItemSkillsList } from '../types.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; import type { CommandContext } from './types.js'; import type { Config, SkillDefinition } from '@google/gemini-cli-core'; -import { SettingScope, type LoadedSettings } from '../../config/settings.js'; +import { + SettingScope, + type LoadedSettings, + createTestMergedSettings, + type MergedSettings, +} from '../../config/settings.js'; vi.mock('../../config/settings.js', async (importOriginal) => { const actual = @@ -55,7 +60,7 @@ describe('skillsCommand', () => { }), } as unknown as Config, settings: { - merged: { skills: { disabled: [] } }, + merged: createTestMergedSettings({ skills: { disabled: [] } }), workspace: { path: '/workspace' }, setValue: vi.fn(), } as unknown as LoadedSettings, @@ -181,7 +186,11 @@ describe('skillsCommand', () => { describe('disable/enable', () => { beforeEach(() => { - context.services.settings.merged.skills = { disabled: [] }; + ( + context.services.settings as unknown as { merged: MergedSettings } + ).merged = createTestMergedSettings({ + skills: { enabled: true, disabled: [] }, + }); ( context.services.settings as unknown as { workspace: { path: string } } ).workspace = { @@ -234,7 +243,14 @@ describe('skillsCommand', () => { const enableCmd = skillsCommand.subCommands!.find( (s) => s.name === 'enable', )!; - context.services.settings.merged.skills = { disabled: ['skill1'] }; + ( + context.services.settings as unknown as { merged: MergedSettings } + ).merged = createTestMergedSettings({ + skills: { + enabled: true, + disabled: ['skill1'], + }, + }); ( context.services.settings as unknown as { workspace: { settings: { skills: { disabled: string[] } } }; diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 79f5a012c0..fe20216219 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1537,6 +1537,13 @@ "default": {}, "type": "object", "properties": { + "enabled": { + "title": "Enable Agent Skills", + "description": "Enable Agent Skills.", + "markdownDescription": "Enable Agent Skills.\n\n- Category: `Advanced`\n- Requires restart: `yes`\n- Default: `true`", + "default": true, + "type": "boolean" + }, "disabled": { "title": "Disabled Skills", "description": "List of disabled skills.", diff --git a/scripts/generate-settings-doc.ts b/scripts/generate-settings-doc.ts index 1511559705..32d9d47c0b 100644 --- a/scripts/generate-settings-doc.ts +++ b/scripts/generate-settings-doc.ts @@ -133,6 +133,10 @@ function collectEntries( definition.properties && Object.keys(definition.properties).length > 0; + if (definition.ignoreInDocs) { + continue; + } + if (!hasChildren && (options.includeAll || definition.showInDialog)) { if (!sections.has(sectionKey)) { sections.set(sectionKey, []);