Support for Built-in Agent Skills (#16045)

This commit is contained in:
N. Taylor Mullen
2026-01-09 22:26:58 -08:00
committed by GitHub
parent b54e688c75
commit 461c277bf2
17 changed files with 755 additions and 451 deletions

View File

@@ -65,7 +65,7 @@ describe('skills list command', () => {
};
mockLoadCliConfig.mockResolvedValue(mockConfig as unknown as Config);
await handleList();
await handleList({});
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
@@ -96,7 +96,7 @@ describe('skills list command', () => {
};
mockLoadCliConfig.mockResolvedValue(mockConfig as unknown as Config);
await handleList();
await handleList({});
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
@@ -120,10 +120,63 @@ describe('skills list command', () => {
);
});
it('should filter built-in skills by default and show them with { all: true }', async () => {
const skills = [
{
name: 'regular',
description: 'desc1',
disabled: false,
location: '/loc1',
},
{
name: 'builtin',
description: 'desc2',
disabled: false,
location: '/loc2',
isBuiltin: true,
},
];
const mockConfig = {
initialize: vi.fn().mockResolvedValue(undefined),
getSkillManager: vi.fn().mockReturnValue({
getAllSkills: vi.fn().mockReturnValue(skills),
}),
};
mockLoadCliConfig.mockResolvedValue(mockConfig as unknown as Config);
// Default
await handleList({ all: false });
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
expect.stringContaining('regular'),
);
expect(emitConsoleLog).not.toHaveBeenCalledWith(
'log',
expect.stringContaining('builtin'),
);
vi.clearAllMocks();
// With all: true
await handleList({ all: true });
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
expect.stringContaining('regular'),
);
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
expect.stringContaining('builtin'),
);
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
expect.stringContaining(chalk.gray(' [Built-in]')),
);
});
it('should throw an error when listing fails', async () => {
mockLoadCliConfig.mockRejectedValue(new Error('List failed'));
await expect(handleList()).rejects.toThrow('List failed');
await expect(handleList({})).rejects.toThrow('List failed');
});
});

View File

@@ -11,7 +11,7 @@ import { loadCliConfig, type CliArgs } from '../../config/config.js';
import { exitCli } from '../utils.js';
import chalk from 'chalk';
export async function handleList() {
export async function handleList(args: { all?: boolean }) {
const workspaceDir = process.cwd();
const settings = loadSettings(workspaceDir);
@@ -28,7 +28,17 @@ export async function handleList() {
await config.initialize();
const skillManager = config.getSkillManager();
const skills = skillManager.getAllSkills();
const skills = args.all
? skillManager.getAllSkills()
: skillManager.getAllSkills().filter((s) => !s.isBuiltin);
// Sort skills: non-built-in first, then alphabetically by name
skills.sort((a, b) => {
if (a.isBuiltin === b.isBuiltin) {
return a.name.localeCompare(b.name);
}
return a.isBuiltin ? 1 : -1;
});
if (skills.length === 0) {
debugLogger.log('No skills discovered.');
@@ -43,7 +53,9 @@ export async function handleList() {
? chalk.red('[Disabled]')
: chalk.green('[Enabled]');
debugLogger.log(`${chalk.bold(skill.name)} ${status}`);
const builtinSuffix = skill.isBuiltin ? chalk.gray(' [Built-in]') : '';
debugLogger.log(`${chalk.bold(skill.name)} ${status}${builtinSuffix}`);
debugLogger.log(` Description: ${skill.description}`);
debugLogger.log(` Location: ${skill.location}`);
debugLogger.log('');
@@ -53,9 +65,14 @@ export async function handleList() {
export const listCommand: CommandModule = {
command: 'list',
describe: 'Lists discovered agent skills.',
builder: (yargs) => yargs,
handler: async () => {
await handleList();
builder: (yargs) =>
yargs.option('all', {
type: 'boolean',
description: 'Show all skills, including built-in ones.',
default: false,
}),
handler: async (argv) => {
await handleList({ all: argv['all'] as boolean });
await exitCli();
},
};

View File

@@ -4,26 +4,27 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { ExtensionManager } from './extension-manager.js';
import { loadSettings } from './settings.js';
import { debugLogger, coreEvents } from '@google/gemini-cli-core';
import { type Settings } from './settings.js';
import { createExtension } from '../test-utils/createExtension.js';
import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js';
import { coreEvents, debugLogger } from '@google/gemini-cli-core';
const mockHomedir = vi.hoisted(() => vi.fn());
const mockHomedir = vi.hoisted(() => vi.fn(() => '/tmp/mock-home'));
vi.mock('os', async (importOriginal) => {
const mockedOs = await importOriginal<typeof import('node:os')>();
vi.mock('node:os', async (importOriginal) => {
const actual = await importOriginal<typeof import('node:os')>();
return {
...mockedOs,
...actual,
homedir: mockHomedir,
};
});
// Mock @google/gemini-cli-core
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@google/gemini-cli-core')>();
@@ -34,92 +35,130 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
});
describe('ExtensionManager skills validation', () => {
let tempHomeDir: string;
let tempWorkspaceDir: string;
let userExtensionsDir: string;
let extensionManager: ExtensionManager;
let tempDir: string;
let extensionsDir: string;
beforeEach(() => {
tempHomeDir = fs.mkdtempSync(
path.join(os.tmpdir(), 'gemini-cli-skills-test-home-'),
);
tempWorkspaceDir = fs.mkdtempSync(
path.join(tempHomeDir, 'gemini-cli-skills-test-workspace-'),
);
userExtensionsDir = path.join(tempHomeDir, EXTENSIONS_DIRECTORY_NAME);
fs.mkdirSync(userExtensionsDir, { recursive: true });
mockHomedir.mockReturnValue(tempHomeDir);
extensionManager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: vi.fn().mockResolvedValue(''),
settings: loadSettings(tempWorkspaceDir).merged,
});
vi.clearAllMocks();
vi.spyOn(coreEvents, 'emitFeedback');
vi.spyOn(debugLogger, 'debug').mockImplementation(() => {});
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-test-'));
mockHomedir.mockReturnValue(tempDir);
// Create the extensions directory that ExtensionManager expects
extensionsDir = path.join(tempDir, '.gemini', EXTENSIONS_DIRECTORY_NAME);
fs.mkdirSync(extensionsDir, { recursive: true });
extensionManager = new ExtensionManager({
settings: {
telemetry: { enabled: false },
trustedFolders: [tempDir],
} as unknown as Settings,
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: vi.fn(),
workspaceDir: tempDir,
});
});
afterEach(() => {
fs.rmSync(tempHomeDir, { recursive: true, force: true });
vi.restoreAllMocks();
try {
fs.rmSync(tempDir, { recursive: true, force: true });
} catch {
// ignore
}
});
it('should emit a warning during install if skills directory is not empty but no skills are loaded', async () => {
const sourceExtDir = createExtension({
extensionsDir: tempHomeDir,
// Create a source extension
const sourceDir = path.join(tempDir, 'source-ext');
createExtension({
extensionsDir: sourceDir, // createExtension appends name
name: 'skills-ext',
version: '1.0.0',
installMetadata: {
type: 'local',
source: path.join(sourceDir, 'skills-ext'),
},
});
const extensionPath = path.join(sourceDir, 'skills-ext');
const skillsDir = path.join(sourceExtDir, 'skills');
// Add invalid skills content
const skillsDir = path.join(extensionPath, 'skills');
fs.mkdirSync(skillsDir);
fs.writeFileSync(path.join(skillsDir, 'not-a-skill.txt'), 'hello');
await extensionManager.loadExtensions();
const extension = await extensionManager.installOrUpdateExtension({
source: sourceExtDir,
await extensionManager.installOrUpdateExtension({
type: 'local',
source: extensionPath,
});
expect(extension.name).toBe('skills-ext');
expect(debugLogger.debug).toHaveBeenCalledWith(
expect.stringContaining('Failed to load skills from'),
);
});
it('should emit a warning during load if skills directory is not empty but no skills are loaded', async () => {
const extDir = createExtension({
extensionsDir: userExtensionsDir,
name: 'load-skills-ext',
// 1. Create a source extension
const sourceDir = path.join(tempDir, 'source-ext-load');
createExtension({
extensionsDir: sourceDir,
name: 'skills-ext-load',
version: '1.0.0',
});
const sourceExtPath = path.join(sourceDir, 'skills-ext-load');
const skillsDir = path.join(extDir, 'skills');
// Add invalid skills content
const skillsDir = path.join(sourceExtPath, 'skills');
fs.mkdirSync(skillsDir);
fs.writeFileSync(path.join(skillsDir, 'not-a-skill.txt'), 'hello');
// 2. Install it to ensure correct disk state
await extensionManager.loadExtensions();
await extensionManager.installOrUpdateExtension({
type: 'local',
source: sourceExtPath,
});
// Clear the spy
vi.mocked(debugLogger.debug).mockClear();
// 3. Create a fresh ExtensionManager to force loading from disk
const newExtensionManager = new ExtensionManager({
settings: {
telemetry: { enabled: false },
trustedFolders: [tempDir],
} as unknown as Settings,
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: vi.fn(),
workspaceDir: tempDir,
});
// 4. Load extensions
await newExtensionManager.loadExtensions();
expect(debugLogger.debug).toHaveBeenCalledWith(
expect.stringContaining('Failed to load skills from'),
);
expect(debugLogger.debug).toHaveBeenCalledWith(
expect.stringContaining(
'The directory is not empty but no valid skills were discovered',
),
);
});
it('should succeed if skills are correctly loaded', async () => {
const sourceExtDir = createExtension({
extensionsDir: tempHomeDir,
const sourceDir = path.join(tempDir, 'source-ext-good');
createExtension({
extensionsDir: sourceDir,
name: 'good-skills-ext',
version: '1.0.0',
installMetadata: {
type: 'local',
source: path.join(sourceDir, 'good-skills-ext'),
},
});
const extensionPath = path.join(sourceDir, 'good-skills-ext');
const skillsDir = path.join(sourceExtDir, 'skills');
const skillsDir = path.join(extensionPath, 'skills');
const skillSubdir = path.join(skillsDir, 'test-skill');
fs.mkdirSync(skillSubdir, { recursive: true });
fs.writeFileSync(
@@ -128,15 +167,13 @@ describe('ExtensionManager skills validation', () => {
);
await extensionManager.loadExtensions();
const extension = await extensionManager.installOrUpdateExtension({
source: sourceExtDir,
type: 'local',
source: extensionPath,
});
expect(extension.skills).toHaveLength(1);
expect(extension.skills![0].name).toBe('test-skill');
// It might be called for other reasons during startup, but shouldn't be called for our skills loading success
// Actually, it shouldn't be called with our warning message
expect(extension.name).toBe('good-skills-ext');
expect(debugLogger.debug).not.toHaveBeenCalledWith(
expect.stringContaining('Failed to load skills from'),
);

View File

@@ -6,7 +6,7 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { skillsCommand } from './skillsCommand.js';
import { MessageType } from '../types.js';
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';
@@ -136,6 +136,51 @@ describe('skillsCommand', () => {
);
});
it('should filter built-in skills by default and show them with "all"', async () => {
const skillManager = context.services.config!.getSkillManager();
const mockSkills = [
{
name: 'regular',
description: 'desc1',
location: '/loc1',
body: 'body1',
},
{
name: 'builtin',
description: 'desc2',
location: '/loc2',
body: 'body2',
isBuiltin: true,
},
];
vi.mocked(skillManager.getAllSkills).mockReturnValue(mockSkills);
const listCmd = skillsCommand.subCommands!.find((s) => s.name === 'list')!;
// By default, only regular skills
await listCmd.action!(context, '');
let lastCall = vi
.mocked(context.ui.addItem)
.mock.calls.at(-1)![0] as HistoryItemSkillsList;
expect(lastCall.skills).toHaveLength(1);
expect(lastCall.skills[0].name).toBe('regular');
// With "all", show both
await listCmd.action!(context, 'all');
lastCall = vi
.mocked(context.ui.addItem)
.mock.calls.at(-1)![0] as HistoryItemSkillsList;
expect(lastCall.skills).toHaveLength(2);
expect(lastCall.skills.map((s) => s.name)).toContain('builtin');
// With "--all", show both
await listCmd.action!(context, '--all');
lastCall = vi
.mocked(context.ui.addItem)
.mock.calls.at(-1)![0] as HistoryItemSkillsList;
expect(lastCall.skills).toHaveLength(2);
});
describe('disable/enable', () => {
beforeEach(() => {
context.services.settings.merged.skills = { disabled: [] };

View File

@@ -23,12 +23,18 @@ async function listAction(
context: CommandContext,
args: string,
): Promise<void | SlashCommandActionReturn> {
const subCommand = args.trim();
const subArgs = args.trim().split(/\s+/);
// Default to SHOWING descriptions. The user can hide them with 'nodesc'.
let useShowDescriptions = true;
if (subCommand === 'nodesc') {
useShowDescriptions = false;
let showAll = false;
for (const arg of subArgs) {
if (arg === 'nodesc' || arg === '--nodesc') {
useShowDescriptions = false;
} else if (arg === 'all' || arg === '--all') {
showAll = true;
}
}
const skillManager = context.services.config?.getSkillManager();
@@ -43,7 +49,9 @@ async function listAction(
return;
}
const skills = skillManager.getAllSkills();
const skills = showAll
? skillManager.getAllSkills()
: skillManager.getAllSkills().filter((s) => !s.isBuiltin);
const skillsListItem: HistoryItemSkillsList = {
type: MessageType.SKILLS_LIST,
@@ -53,6 +61,7 @@ async function listAction(
disabled: skill.disabled,
location: skill.location,
body: skill.body,
isBuiltin: skill.isBuiltin,
})),
showDescriptions: useShowDescriptions,
};
@@ -278,7 +287,8 @@ export const skillsCommand: SlashCommand = {
subCommands: [
{
name: 'list',
description: 'List available agent skills. Usage: /skills list [nodesc]',
description:
'List available agent skills. Usage: /skills list [nodesc] [all]',
kind: CommandKind.BUILT_IN,
action: listAction,
},

View File

@@ -154,6 +154,7 @@ const createMockConfig = (overrides = {}) => ({
}),
getSkillManager: () => ({
getSkills: () => [],
getDisplayableSkills: () => [],
}),
getMcpClientManager: () => ({
getMcpServers: () => ({}),

View File

@@ -45,6 +45,7 @@ const createMockConfig = (overrides = {}) => ({
})),
getSkillManager: vi.fn().mockImplementation(() => ({
getSkills: vi.fn(() => ['skill1', 'skill2']),
getDisplayableSkills: vi.fn(() => ['skill1', 'skill2']),
})),
...overrides,
});

View File

@@ -25,7 +25,7 @@ export const StatusDisplay: React.FC<StatusDisplayProps> = ({
const config = useConfig();
if (process.env['GEMINI_SYSTEM_MD']) {
return <Text color={theme.status.error}>|_| </Text>;
return <Text color={theme.status.error}>|_|</Text>;
}
if (uiState.ctrlCPressedOnce) {
@@ -69,7 +69,7 @@ export const StatusDisplay: React.FC<StatusDisplayProps> = ({
blockedMcpServers={
config.getMcpClientManager()?.getBlockedMcpServers() ?? []
}
skillCount={config.getSkillManager().getSkills().length}
skillCount={config.getSkillManager().getDisplayableSkills().length}
/>
);
}

View File

@@ -105,4 +105,25 @@ describe('SkillsList Component', () => {
unmount();
});
it('should render [Built-in] tag for built-in skills', () => {
const builtinSkill: SkillDefinition = {
name: 'builtin-skill',
description: 'A built-in skill',
disabled: false,
location: 'loc',
body: 'body',
isBuiltin: true,
};
const { lastFrame, unmount } = render(
<SkillsList skills={[builtinSkill]} showDescriptions={true} />,
);
const output = lastFrame();
expect(output).toContain('builtin-skill');
expect(output).toContain('Built-in');
unmount();
});
});

View File

@@ -18,24 +18,32 @@ export const SkillsList: React.FC<SkillsListProps> = ({
skills,
showDescriptions,
}) => {
const enabledSkills = skills
.filter((s) => !s.disabled)
.sort((a, b) => a.name.localeCompare(b.name));
const sortSkills = (a: SkillDefinition, b: SkillDefinition) => {
if (a.isBuiltin === b.isBuiltin) {
return a.name.localeCompare(b.name);
}
return a.isBuiltin ? 1 : -1;
};
const disabledSkills = skills
.filter((s) => s.disabled)
.sort((a, b) => a.name.localeCompare(b.name));
const enabledSkills = skills.filter((s) => !s.disabled).sort(sortSkills);
const disabledSkills = skills.filter((s) => s.disabled).sort(sortSkills);
const renderSkill = (skill: SkillDefinition) => (
<Box key={skill.name} flexDirection="row">
<Text color={theme.text.primary}>{' '}- </Text>
<Box flexDirection="column">
<Text
bold
color={skill.disabled ? theme.text.secondary : theme.text.link}
>
{skill.name}
</Text>
<Box flexDirection="row">
<Text
bold
color={skill.disabled ? theme.text.secondary : theme.text.link}
>
{skill.name}
</Text>
{skill.isBuiltin && (
<Text color={theme.text.secondary}>{' [Built-in]'}</Text>
)}
</Box>
{showDescriptions && skill.description && (
<Box marginLeft={2}>
<Text

View File

@@ -71,6 +71,7 @@ describe('useQuotaAndFallback', () => {
vi.spyOn(mockConfig, 'setQuotaErrorOccurred');
vi.spyOn(mockConfig, 'setModel');
vi.spyOn(mockConfig, 'setActiveModel');
vi.spyOn(mockConfig, 'activateFallbackMode');
});
afterEach(() => {
@@ -165,8 +166,10 @@ describe('useQuotaAndFallback', () => {
const intent = await promise!;
expect(intent).toBe('retry_always');
// Verify setActiveModel was called
expect(mockConfig.setActiveModel).toHaveBeenCalledWith('gemini-flash');
// Verify activateFallbackMode was called
expect(mockConfig.activateFallbackMode).toHaveBeenCalledWith(
'gemini-flash',
);
// The pending request should be cleared from the state
expect(result.current.proQuotaRequest).toBeNull();
@@ -279,8 +282,10 @@ describe('useQuotaAndFallback', () => {
const intent = await promise!;
expect(intent).toBe('retry_always');
// Verify setActiveModel was called
expect(mockConfig.setActiveModel).toHaveBeenCalledWith('model-B');
// Verify activateFallbackMode was called
expect(mockConfig.activateFallbackMode).toHaveBeenCalledWith(
'model-B',
);
// The pending request should be cleared from the state
expect(result.current.proQuotaRequest).toBeNull();
@@ -337,8 +342,8 @@ To disable gemini-3-pro-preview, disable "Preview features" in /settings.`,
const intent = await promise!;
expect(intent).toBe('retry_always');
// Verify setActiveModel was called
expect(mockConfig.setActiveModel).toHaveBeenCalledWith(
// Verify activateFallbackMode was called
expect(mockConfig.activateFallbackMode).toHaveBeenCalledWith(
'gemini-2.5-pro',
);
@@ -425,8 +430,10 @@ To disable gemini-3-pro-preview, disable "Preview features" in /settings.`,
expect(intent).toBe('retry_always');
expect(result.current.proQuotaRequest).toBeNull();
// Verify setActiveModel was called
expect(mockConfig.setActiveModel).toHaveBeenCalledWith('gemini-flash');
// Verify activateFallbackMode was called
expect(mockConfig.activateFallbackMode).toHaveBeenCalledWith(
'gemini-flash',
);
// Verify quota error flags are reset
expect(mockSetModelSwitchedFromQuotaError).toHaveBeenCalledWith(false);

View File

@@ -4,219 +4,204 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import fs from 'node:fs/promises';
import {
calculateTurnStats,
calculateRewindImpact,
revertFileChanges,
} from './rewindFileOps.js';
import type {
ConversationRecord,
MessageRecord,
ToolCallRecord,
import {
coreEvents,
type ConversationRecord,
type MessageRecord,
type ToolCallRecord,
} from '@google/gemini-cli-core';
import { coreEvents } from '@google/gemini-cli-core';
import fs from 'node:fs/promises';
import path from 'node:path';
vi.mock('node:fs/promises');
// Mock fs/promises
vi.mock('node:fs/promises', () => ({
default: {
readFile: vi.fn(),
writeFile: vi.fn(),
rm: vi.fn(),
unlink: vi.fn(),
},
}));
// Mock @google/gemini-cli-core
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@google/gemini-cli-core')>();
return {
...actual,
coreEvents: {
emitFeedback: vi.fn(),
debugLogger: {
log: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
},
getFileDiffFromResultDisplay: vi.fn(),
computeAddedAndRemovedLines: vi.fn(),
};
});
describe('rewindFileOps', () => {
const mockConversation: ConversationRecord = {
sessionId: 'test-session',
projectHash: 'hash',
startTime: 'time',
lastUpdated: 'time',
messages: [],
};
beforeEach(() => {
vi.clearAllMocks();
});
afterEach(() => {
vi.restoreAllMocks();
vi.spyOn(coreEvents, 'emitFeedback');
});
describe('calculateTurnStats', () => {
it('returns null if no edits found after user message', () => {
const userMsg: MessageRecord = {
type: 'user',
content: 'hello',
id: '1',
timestamp: '1',
const userMsg = { type: 'user' } as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{ type: 'gemini', text: 'Hello' } as unknown as MessageRecord,
],
};
const geminiMsg: MessageRecord = {
type: 'gemini',
content: 'hi',
id: '2',
timestamp: '2',
};
mockConversation.messages = [userMsg, geminiMsg];
const stats = calculateTurnStats(mockConversation, userMsg);
expect(stats).toBeNull();
const result = calculateTurnStats(
conversation as unknown as ConversationRecord,
userMsg,
);
expect(result).toBeNull();
});
it('calculates stats for single turn correctly', () => {
const userMsg: MessageRecord = {
type: 'user',
content: 'hello',
id: '1',
timestamp: '1',
};
const toolMsg: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
it('calculates stats for single turn correctly', async () => {
const { getFileDiffFromResultDisplay, computeAddedAndRemovedLines } =
await import('@google/gemini-cli-core');
vi.mocked(getFileDiffFromResultDisplay).mockReturnValue({
filePath: 'test.ts',
fileName: 'test.ts',
originalContent: 'old',
newContent: 'new',
isNewFile: false,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
fileDiff: 'diff',
});
vi.mocked(computeAddedAndRemovedLines).mockReturnValue({
addedLines: 3,
removedLines: 3,
});
const userMsg = { type: 'user' } as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
name: 'replace',
id: 'tool-call-1',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file1.ts',
filePath: '/file1.ts',
originalContent: 'old',
newContent: 'new',
isNewFile: false,
diffStat: {
model_added_lines: 5,
model_removed_lines: 2,
user_added_lines: 0,
user_removed_lines: 0,
model_added_chars: 100,
model_removed_chars: 20,
user_added_chars: 0,
user_removed_chars: 0,
type: 'gemini',
toolCalls: [
{
name: 'replace',
args: {},
resultDisplay: 'diff',
},
},
},
] as unknown as ToolCallRecord[],
],
} as unknown as MessageRecord,
],
};
const userMsg2: MessageRecord = {
type: 'user',
content: 'next',
id: '3',
timestamp: '3',
};
mockConversation.messages = [userMsg, toolMsg, userMsg2];
const stats = calculateTurnStats(mockConversation, userMsg);
expect(stats).toEqual({
addedLines: 5,
removedLines: 2,
const result = calculateTurnStats(
conversation as unknown as ConversationRecord,
userMsg,
);
expect(result).toEqual({
fileCount: 1,
addedLines: 3,
removedLines: 3,
});
});
});
describe('calculateRewindImpact', () => {
it('calculates cumulative stats across multiple turns', () => {
const userMsg1: MessageRecord = {
type: 'user',
content: 'start',
id: '1',
timestamp: '1',
};
const toolMsg1: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
{
name: 'replace',
id: 'tool-call-1',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file1.ts',
filePath: '/file1.ts',
fileDiff: 'diff1',
originalContent: 'old',
newContent: 'new',
isNewFile: false,
diffStat: {
model_added_lines: 5,
model_removed_lines: 2,
user_added_lines: 0,
user_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
},
it('calculates cumulative stats across multiple turns', async () => {
const { getFileDiffFromResultDisplay, computeAddedAndRemovedLines } =
await import('@google/gemini-cli-core');
vi.mocked(getFileDiffFromResultDisplay)
.mockReturnValueOnce({
filePath: 'file1.ts',
fileName: 'file1.ts',
originalContent: '123',
newContent: '12345',
isNewFile: false,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
] as unknown as ToolCallRecord[],
};
const userMsg2: MessageRecord = {
type: 'user',
content: 'next',
id: '3',
timestamp: '3',
};
const toolMsg2: MessageRecord = {
type: 'gemini',
id: '4',
timestamp: '4',
content: '',
toolCalls: [
{
name: 'replace',
id: 'tool-call-2',
status: 'success',
timestamp: '4',
args: {},
resultDisplay: {
fileName: 'file2.ts',
filePath: '/file2.ts',
fileDiff: 'diff2',
originalContent: 'old',
newContent: 'new',
isNewFile: false,
diffStat: {
model_added_lines: 3,
model_removed_lines: 1,
user_added_lines: 0,
user_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
},
fileDiff: 'diff1',
})
.mockReturnValueOnce({
filePath: 'file2.ts',
fileName: 'file2.ts',
originalContent: 'abc',
newContent: 'abcd',
isNewFile: true,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
] as unknown as ToolCallRecord[],
fileDiff: 'diff2',
});
vi.mocked(computeAddedAndRemovedLines)
.mockReturnValueOnce({ addedLines: 5, removedLines: 3 })
.mockReturnValueOnce({ addedLines: 4, removedLines: 0 });
const userMsg = { type: 'user' } as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
type: 'gemini',
toolCalls: [
{
resultDisplay: 'd1',
} as unknown as ToolCallRecord,
],
} as unknown as MessageRecord,
{
type: 'user',
} as unknown as MessageRecord,
{
type: 'gemini',
toolCalls: [
{
resultDisplay: 'd2',
} as unknown as ToolCallRecord,
],
} as unknown as MessageRecord,
],
};
mockConversation.messages = [userMsg1, toolMsg1, userMsg2, toolMsg2];
const stats = calculateRewindImpact(mockConversation, userMsg1);
expect(stats).toEqual({
addedLines: 8, // 5 + 3
removedLines: 3, // 2 + 1
const result = calculateRewindImpact(
conversation as unknown as ConversationRecord,
userMsg,
);
expect(result).toEqual({
fileCount: 2,
addedLines: 9, // 5 + 4
removedLines: 3, // 3 + 0
details: [
{ fileName: 'file1.ts', diff: 'diff1' },
{ fileName: 'file2.ts', diff: 'diff2' },
@@ -226,246 +211,264 @@ describe('rewindFileOps', () => {
});
describe('revertFileChanges', () => {
const mockDiffStat = {
model_added_lines: 1,
model_removed_lines: 1,
user_added_lines: 0,
user_removed_lines: 0,
model_added_chars: 1,
model_removed_chars: 1,
user_added_chars: 0,
user_removed_chars: 0,
};
it('does nothing if message not found', async () => {
mockConversation.messages = [];
await revertFileChanges(mockConversation, 'missing-id');
await revertFileChanges(
{ messages: [] } as unknown as ConversationRecord,
'missing',
);
expect(fs.writeFile).not.toHaveBeenCalled();
});
it('reverts exact match', async () => {
const userMsg: MessageRecord = {
const { getFileDiffFromResultDisplay } = await import(
'@google/gemini-cli-core'
);
vi.mocked(getFileDiffFromResultDisplay).mockReturnValue({
filePath: '/abs/path/test.ts',
fileName: 'test.ts',
originalContent: 'ORIGINAL_CONTENT',
newContent: 'NEW_CONTENT',
isNewFile: false,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
fileDiff: 'diff',
});
const userMsg = {
type: 'user',
content: 'start',
id: '1',
timestamp: '1',
};
const toolMsg: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
id: 'target',
} as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
name: 'replace',
id: 'tool-call-1',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file.txt',
filePath: path.resolve('/root/file.txt'),
originalContent: 'old',
newContent: 'new',
isNewFile: false,
diffStat: mockDiffStat,
},
},
] as unknown as ToolCallRecord[],
type: 'gemini',
toolCalls: [{ resultDisplay: 'diff' } as unknown as ToolCallRecord],
} as unknown as MessageRecord,
],
};
mockConversation.messages = [userMsg, toolMsg];
vi.mocked(fs.readFile).mockResolvedValue('NEW_CONTENT');
vi.mocked(fs.readFile).mockResolvedValue('new');
await revertFileChanges(mockConversation, '1');
await revertFileChanges(
conversation as unknown as ConversationRecord,
'target',
);
expect(fs.writeFile).toHaveBeenCalledWith(
path.resolve('/root/file.txt'),
'old',
'/abs/path/test.ts',
'ORIGINAL_CONTENT',
);
});
it('deletes new file on revert', async () => {
const userMsg: MessageRecord = {
const { getFileDiffFromResultDisplay } = await import(
'@google/gemini-cli-core'
);
vi.mocked(getFileDiffFromResultDisplay).mockReturnValue({
filePath: '/abs/path/new.ts',
fileName: 'new.ts',
originalContent: '',
newContent: 'SOME_CONTENT',
isNewFile: true,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
fileDiff: 'diff',
});
const userMsg = {
type: 'user',
content: 'start',
id: '1',
timestamp: '1',
};
const toolMsg: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
id: 'target',
} as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
name: 'write_file',
id: 'tool-call-2',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file.txt',
filePath: path.resolve('/root/file.txt'),
originalContent: null,
newContent: 'content',
isNewFile: true,
diffStat: mockDiffStat,
},
},
] as unknown as ToolCallRecord[],
type: 'gemini',
toolCalls: [{ resultDisplay: 'diff' } as unknown as ToolCallRecord],
} as unknown as MessageRecord,
],
};
mockConversation.messages = [userMsg, toolMsg];
vi.mocked(fs.readFile).mockResolvedValue('SOME_CONTENT');
vi.mocked(fs.readFile).mockResolvedValue('content');
await revertFileChanges(
conversation as unknown as ConversationRecord,
'target',
);
await revertFileChanges(mockConversation, '1');
expect(fs.unlink).toHaveBeenCalledWith(path.resolve('/root/file.txt'));
expect(fs.unlink).toHaveBeenCalledWith('/abs/path/new.ts');
});
it('handles smart revert (patching) successfully', async () => {
const original = Array.from(
{ length: 20 },
(_, i) => `line${i + 1}`,
).join('\n');
// Agent changes line 2
const agentModifiedLines = original.split('\n');
agentModifiedLines[1] = 'line2-modified';
const agentModified = agentModifiedLines.join('\n');
const { getFileDiffFromResultDisplay } = await import(
'@google/gemini-cli-core'
);
vi.mocked(getFileDiffFromResultDisplay).mockReturnValue({
filePath: '/abs/path/test.ts',
fileName: 'test.ts',
originalContent: 'LINE1\nLINE2\nLINE3',
newContent: 'LINE1\nEDITED\nLINE3',
isNewFile: false,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
fileDiff: 'diff',
});
// User changes line 18 (far away from line 2)
const userModifiedLines = [...agentModifiedLines];
userModifiedLines[17] = 'line18-modified';
const userModified = userModifiedLines.join('\n');
const toolMsg: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
const userMsg = {
type: 'user',
id: 'target',
} as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
name: 'replace',
id: 'tool-call-1',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file.txt',
filePath: path.resolve('/root/file.txt'),
originalContent: original,
newContent: agentModified,
isNewFile: false,
diffStat: mockDiffStat,
},
},
] as unknown as ToolCallRecord[],
type: 'gemini',
toolCalls: [{ resultDisplay: 'diff' } as unknown as ToolCallRecord],
} as unknown as MessageRecord,
],
};
mockConversation.messages = [
{ type: 'user', content: 'start', id: '1', timestamp: '1' },
toolMsg,
];
vi.mocked(fs.readFile).mockResolvedValue(userModified);
// Current content has FURTHER changes
vi.mocked(fs.readFile).mockResolvedValue('LINE1\nEDITED\nLINE3\nNEWLINE');
await revertFileChanges(mockConversation, '1');
// Expect line 2 to be reverted to original, but line 18 to keep user modification
const expectedLines = original.split('\n');
expectedLines[17] = 'line18-modified';
const expectedContent = expectedLines.join('\n');
await revertFileChanges(
conversation as unknown as ConversationRecord,
'target',
);
// Should have successfully patched it back to ORIGINAL state but kept the NEWLINE
expect(fs.writeFile).toHaveBeenCalledWith(
path.resolve('/root/file.txt'),
expectedContent,
'/abs/path/test.ts',
'LINE1\nLINE2\nLINE3\nNEWLINE',
);
});
it('emits warning on smart revert failure', async () => {
const original = 'line1\nline2\nline3';
const agentModified = 'line1\nline2-modified\nline3';
// User modification conflicts with the agent's change.
const userModified = 'line1\nline2-usermodified\nline3';
const { getFileDiffFromResultDisplay } = await import(
'@google/gemini-cli-core'
);
vi.mocked(getFileDiffFromResultDisplay).mockReturnValue({
filePath: '/abs/path/test.ts',
fileName: 'test.ts',
originalContent: 'OLD',
newContent: 'NEW',
isNewFile: false,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
fileDiff: 'diff',
});
const toolMsg: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
const userMsg = {
type: 'user',
id: 'target',
} as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
name: 'replace',
id: 'tool-call-1',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file.txt',
filePath: path.resolve('/root/file.txt'),
originalContent: original,
newContent: agentModified,
isNewFile: false,
diffStat: mockDiffStat,
},
},
] as unknown as ToolCallRecord[],
type: 'gemini',
toolCalls: [{ resultDisplay: 'diff' } as unknown as ToolCallRecord],
} as unknown as MessageRecord,
],
};
mockConversation.messages = [
{ type: 'user', content: 'start', id: '1', timestamp: '1' },
toolMsg,
];
vi.mocked(fs.readFile).mockResolvedValue(userModified);
// Current content is completely unrelated - diff won't apply
vi.mocked(fs.readFile).mockResolvedValue('UNRELATED');
await revertFileChanges(mockConversation, '1');
await revertFileChanges(
conversation as unknown as ConversationRecord,
'target',
);
expect(fs.writeFile).not.toHaveBeenCalled();
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
'warning',
expect.stringContaining('Smart revert for file.txt failed'),
expect.stringContaining('Smart revert for test.ts failed'),
);
});
it('emits error if fs.readFile fails with a generic error', async () => {
const toolMsg: MessageRecord = {
type: 'gemini',
id: '2',
timestamp: '2',
content: '',
toolCalls: [
const { getFileDiffFromResultDisplay } = await import(
'@google/gemini-cli-core'
);
vi.mocked(getFileDiffFromResultDisplay).mockReturnValue({
filePath: '/abs/path/test.ts',
fileName: 'test.ts',
originalContent: 'OLD',
newContent: 'NEW',
isNewFile: false,
diffStat: {
model_added_lines: 0,
model_removed_lines: 0,
model_added_chars: 0,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
fileDiff: 'diff',
});
const userMsg = {
type: 'user',
id: 'target',
} as unknown as MessageRecord;
const conversation = {
messages: [
userMsg,
{
name: 'replace',
id: 'tool-call-1',
status: 'success',
timestamp: '2',
args: {},
resultDisplay: {
fileName: 'file.txt',
filePath: path.resolve('/root/file.txt'),
originalContent: 'old',
newContent: 'new',
isNewFile: false,
diffStat: mockDiffStat,
},
},
] as unknown as ToolCallRecord[],
type: 'gemini',
toolCalls: [{ resultDisplay: 'diff' } as unknown as ToolCallRecord],
} as unknown as MessageRecord,
],
};
mockConversation.messages = [
{ type: 'user', content: 'start', id: '1', timestamp: '1' },
toolMsg,
];
// Simulate a generic file read error
vi.mocked(fs.readFile).mockRejectedValue(new Error('Permission denied'));
vi.mocked(fs.readFile).mockRejectedValue(new Error('disk failure'));
await revertFileChanges(
conversation as unknown as ConversationRecord,
'target',
);
await revertFileChanges(mockConversation, '1');
expect(fs.writeFile).not.toHaveBeenCalled();
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
'error',
'Error reading file.txt during revert: Permission denied',
expect.stringContaining(
'Error reading test.ts during revert: disk failure',
),
expect.any(Error),
);
});

View File

@@ -25,6 +25,8 @@ export interface SkillDefinition {
body: string;
/** Whether the skill is currently disabled. */
disabled?: boolean;
/** Whether the skill is a built-in skill. */
isBuiltin?: boolean;
}
const FRONTMATTER_REGEX = /^---\r?\n([\s\S]*?)\r?\n---\r?\n([\s\S]*)/;

View File

@@ -163,4 +163,45 @@ description: desc1
expect(service.getAllSkills()).toHaveLength(1);
expect(service.getAllSkills()[0].disabled).toBe(true);
});
it('should filter built-in skills in getDisplayableSkills', async () => {
const service = new SkillManager();
// @ts-expect-error accessing private property for testing
service.skills = [
{
name: 'regular-skill',
description: 'regular',
location: 'loc1',
body: 'body',
isBuiltin: false,
},
{
name: 'builtin-skill',
description: 'builtin',
location: 'loc2',
body: 'body',
isBuiltin: true,
},
{
name: 'disabled-builtin',
description: 'disabled builtin',
location: 'loc3',
body: 'body',
isBuiltin: true,
disabled: true,
},
];
const displayable = service.getDisplayableSkills();
expect(displayable).toHaveLength(1);
expect(displayable[0].name).toBe('regular-skill');
const all = service.getAllSkills();
expect(all).toHaveLength(3);
const enabled = service.getSkills();
expect(enabled).toHaveLength(2);
expect(enabled.map((s) => s.name)).toContain('builtin-skill');
});
});

View File

@@ -31,24 +31,36 @@ export class SkillManager {
): Promise<void> {
this.clearSkills();
// 1. Extension skills (lowest precedence)
// 1. Built-in skills (lowest precedence)
await this.discoverBuiltinSkills();
// 2. Extension skills
for (const extension of extensions) {
if (extension.isActive && extension.skills) {
this.addSkillsWithPrecedence(extension.skills);
}
}
// 2. User skills
// 3. User skills
const userSkills = await loadSkillsFromDir(Storage.getUserSkillsDir());
this.addSkillsWithPrecedence(userSkills);
// 3. Project skills (highest precedence)
// 4. Project skills (highest precedence)
const projectSkills = await loadSkillsFromDir(
storage.getProjectSkillsDir(),
);
this.addSkillsWithPrecedence(projectSkills);
}
/**
* Discovers built-in skills.
*/
private async discoverBuiltinSkills(): Promise<void> {
// Built-in skills can be added here.
// For now, this is a placeholder for where built-in skills will be loaded from.
// They could be loaded from a specific directory within the package.
}
private addSkillsWithPrecedence(newSkills: SkillDefinition[]): void {
const skillMap = new Map<string, SkillDefinition>();
for (const skill of [...this.skills, ...newSkills]) {
@@ -64,6 +76,14 @@ export class SkillManager {
return this.skills.filter((s) => !s.disabled);
}
/**
* Returns the list of enabled discovered skills that should be displayed in the UI.
* This excludes built-in skills.
*/
getDisplayableSkills(): SkillDefinition[] {
return this.skills.filter((s) => !s.disabled && !s.isBuiltin);
}
/**
* Returns all discovered skills, including disabled ones.
*/
@@ -82,8 +102,11 @@ export class SkillManager {
* Sets the list of disabled skill names.
*/
setDisabledSkills(disabledNames: string[]): void {
const lowercaseDisabledNames = disabledNames.map((n) => n.toLowerCase());
for (const skill of this.skills) {
skill.disabled = disabledNames.includes(skill.name);
skill.disabled = lowercaseDisabledNames.includes(
skill.name.toLowerCase(),
);
}
}
@@ -91,7 +114,10 @@ export class SkillManager {
* Reads the full content (metadata + body) of a skill by name.
*/
getSkill(name: string): SkillDefinition | null {
return this.skills.find((s) => s.name === name) ?? null;
const lowercaseName = name.toLowerCase();
return (
this.skills.find((s) => s.name.toLowerCase() === lowercaseName) ?? null
);
}
/**

View File

@@ -76,6 +76,34 @@ describe('ActivateSkillTool', () => {
expect(details.prompt).toContain('Mock folder structure');
});
it('should skip confirmation for built-in skills', async () => {
const builtinSkill = {
name: 'builtin-skill',
description: 'A built-in skill',
location: '/path/to/builtin/SKILL.md',
isBuiltin: true,
body: 'Built-in instructions',
};
vi.mocked(mockConfig.getSkillManager().getSkill).mockReturnValue(
builtinSkill,
);
vi.mocked(mockConfig.getSkillManager().getSkills).mockReturnValue([
builtinSkill,
]);
const params = { name: 'builtin-skill' };
const toolWithBuiltin = new ActivateSkillTool(mockConfig, mockMessageBus);
const invocation = toolWithBuiltin.build(params);
const details = await (
invocation as unknown as {
getConfirmationDetails: (signal: AbortSignal) => Promise<unknown>;
}
).getConfirmationDetails(new AbortController().signal);
expect(details).toBe(false);
});
it('should activate a valid skill and return its content in XML tags', async () => {
const params = { name: 'test-skill' };
const invocation = tool.build(params);

View File

@@ -80,6 +80,10 @@ class ActivateSkillToolInvocation extends BaseToolInvocation<
return false;
}
if (skill.isBuiltin) {
return false;
}
const folderStructure = await this.getOrFetchFolderStructure(
skill.location,
);