diff --git a/src/extension/tools/common/toolUtils.ts b/src/extension/tools/common/toolUtils.ts index 162965cb71..01f0455594 100644 --- a/src/extension/tools/common/toolUtils.ts +++ b/src/extension/tools/common/toolUtils.ts @@ -6,12 +6,21 @@ import { URI } from '../../../util/vs/base/common/uri'; import { Location } from '../../../vscodeTypes'; -export function formatUriForFileWidget(uriOrLocation: URI | Location): string { +type FileUriMetadata = { + vscodeLinkType: 'skill'; + linkText: string; +}; + +export function formatUriForFileWidget(uriOrLocation: URI | Location, metadata?: FileUriMetadata): string { const uri = URI.isUri(uriOrLocation) ? uriOrLocation : uriOrLocation.uri; const rangePart = URI.isUri(uriOrLocation) ? '' : `#${uriOrLocation.range.start.line + 1}-${uriOrLocation.range.end.line + 1}`; - // Empty link text -> rendered as file widget + if (metadata) { + const uriWithQuery = uri.with({ query: `vscodeLinkType=${metadata.vscodeLinkType}` }); + return `[${metadata.linkText}](${uriWithQuery.toString()}${rangePart})`; + } + return `[](${uri.toString()}${rangePart})`; } diff --git a/src/extension/tools/node/findTestsFilesTool.tsx b/src/extension/tools/node/findTestsFilesTool.tsx index eade86e5f1..d65c0ba077 100644 --- a/src/extension/tools/node/findTestsFilesTool.tsx +++ b/src/extension/tools/node/findTestsFilesTool.tsx @@ -146,7 +146,7 @@ class FindTestFilesTool extends Disposable implements ICopilotTool formatUriForFileWidget(uri)).join(', '); } async provideInput(promptContext: IBuildPromptContext): Promise { diff --git a/src/extension/tools/node/getErrorsTool.tsx b/src/extension/tools/node/getErrorsTool.tsx index 2cdde1ffd1..b3668f68aa 100644 --- a/src/extension/tools/node/getErrorsTool.tsx +++ b/src/extension/tools/node/getErrorsTool.tsx @@ -236,7 +236,7 @@ export class GetErrorsTool extends Disposable implements ICopilotTool formatUriForFileWidget(uri)).join(', '); } private getNotebookCellDiagnostics(uri: URI) { diff --git a/src/extension/tools/node/readFileTool.tsx b/src/extension/tools/node/readFileTool.tsx index ddcbe11f70..f06a6cb24a 100644 --- a/src/extension/tools/node/readFileTool.tsx +++ b/src/extension/tools/node/readFileTool.tsx @@ -171,7 +171,23 @@ export class ReadFileTool implements ICopilotTool { } const { start, end } = getParamRanges(input, documentSnapshot); + const skillInfo = this.customInstructionsService.getSkillInfo(uri); + if (start === 1 && end === documentSnapshot.lineCount) { + if (skillInfo) { + const { skillName } = skillInfo; + if (this.customInstructionsService.isSkillMdFile(uri)) { + return { + invocationMessage: new MarkdownString(l10n.t`Reading skill ${formatUriForFileWidget(uri, { vscodeLinkType: 'skill', linkText: skillName })}`), + pastTenseMessage: new MarkdownString(l10n.t`Read skill ${formatUriForFileWidget(uri, { vscodeLinkType: 'skill', linkText: skillName })}`), + }; + } else { + return { + invocationMessage: new MarkdownString(l10n.t`Reading skill \`${skillName}\`: ${formatUriForFileWidget(uri)}`), + pastTenseMessage: new MarkdownString(l10n.t`Read skill \`${skillName}\`: ${formatUriForFileWidget(uri)}`), + }; + } + } return { invocationMessage: new MarkdownString(l10n.t`Reading ${formatUriForFileWidget(uri)}`), pastTenseMessage: new MarkdownString(l10n.t`Read ${formatUriForFileWidget(uri)}`), @@ -180,6 +196,22 @@ export class ReadFileTool implements ICopilotTool { // Jump to the start of the range, don't select the whole range const readLocation = new Location(uri, new Range(start - 1, 0, start - 1, 0)); + if (this.customInstructionsService.isSkillFile(uri)) { + if (skillInfo) { + const { skillName } = skillInfo; + if (this.customInstructionsService.isSkillMdFile(uri)) { + return { + invocationMessage: new MarkdownString(l10n.t`Reading skill ${formatUriForFileWidget(readLocation, { vscodeLinkType: 'skill', linkText: skillName })}, lines ${start} to ${end}`), + pastTenseMessage: new MarkdownString(l10n.t`Read skill ${formatUriForFileWidget(readLocation, { vscodeLinkType: 'skill', linkText: skillName })}, lines ${start} to ${end}`), + }; + } else { + return { + invocationMessage: new MarkdownString(l10n.t`Reading skill \`${skillName}\`: ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`), + pastTenseMessage: new MarkdownString(l10n.t`Read skill \`${skillName}\`: ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`), + }; + } + } + } return { invocationMessage: new MarkdownString(l10n.t`Reading ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`), pastTenseMessage: new MarkdownString(l10n.t`Read ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`), diff --git a/src/extension/tools/node/test/editFileToolUtils.spec.ts b/src/extension/tools/node/test/editFileToolUtils.spec.ts index 19611c7aac..ee39a36d08 100644 --- a/src/extension/tools/node/test/editFileToolUtils.spec.ts +++ b/src/extension/tools/node/test/editFileToolUtils.spec.ts @@ -8,10 +8,10 @@ import { homedir } from 'os'; import { afterEach, beforeEach, describe, expect, test } from 'vitest'; import { DefaultsOnlyConfigurationService } from '../../../../platform/configuration/common/defaultsOnlyConfigurationService'; import { InMemoryConfigurationService } from '../../../../platform/configuration/test/common/inMemoryConfigurationService'; -import type { ICustomInstructionsService } from '../../../../platform/customInstructions/common/customInstructionsService'; import { IAlternativeNotebookContentService } from '../../../../platform/notebook/common/alternativeContent'; import { MockAlternativeNotebookContentService } from '../../../../platform/notebook/common/mockAlternativeContentService'; import { INotebookService } from '../../../../platform/notebook/common/notebookService'; +import { MockCustomInstructionsService } from '../../../../platform/test/common/testCustomInstructionsService'; import { TestWorkspaceService } from '../../../../platform/test/node/testWorkspaceService'; import { WorkspaceEdit as WorkspaceEditShim } from '../../../../util/common/test/shims/editing'; import { createTextDocumentData, IExtHostDocumentData, setDocText } from '../../../../util/common/test/shims/textDocument'; @@ -654,41 +654,6 @@ describe('assertPathIsSafe (Windows scenarios)', () => { }); describe('makeUriConfirmationChecker', async () => { - // Mock custom instructions service - class MockCustomInstructionsService implements ICustomInstructionsService { - declare readonly _serviceBrand: undefined; - private externalFiles = new Set(); - - setExternalFiles(uris: URI[]) { - this.externalFiles.clear(); - uris.forEach(uri => this.externalFiles.add(uri.toString())); - } - - isExternalInstructionsFile(uri: URI): boolean { - return this.externalFiles.has(uri.toString()); - } - - isExternalInstructionsFolder(uri: URI): boolean { - return false; - } - - isSkillFile(uri: URI): boolean { - return false; - } - - fetchInstructionsFromSetting(): Promise { - return Promise.resolve([]); - } - - fetchInstructionsFromFile(): Promise { - return Promise.resolve(undefined); - } - - getAgentInstructions(): Promise { - return Promise.resolve([]); - } - } - let configService: InMemoryConfigurationService; let workspaceService: TestWorkspaceService; let customInstructionsService: MockCustomInstructionsService; diff --git a/src/extension/tools/node/test/readFile.spec.tsx b/src/extension/tools/node/test/readFile.spec.tsx index 5d3f8716b7..f3c96537bb 100644 --- a/src/extension/tools/node/test/readFile.spec.tsx +++ b/src/extension/tools/node/test/readFile.spec.tsx @@ -4,6 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { afterAll, beforeAll, expect, suite, test } from 'vitest'; +import { ICustomInstructionsService } from '../../../../platform/customInstructions/common/customInstructionsService'; +import { MockCustomInstructionsService } from '../../../../platform/test/common/testCustomInstructionsService'; import { ITestingServicesAccessor } from '../../../../platform/test/node/services'; import { TestWorkspaceService } from '../../../../platform/test/node/testWorkspaceService'; import { IWorkspaceService } from '../../../../platform/workspace/common/workspaceService'; @@ -11,10 +13,12 @@ import { createTextDocumentData } from '../../../../util/common/test/shims/textD import { CancellationToken } from '../../../../util/vs/base/common/cancellation'; import { URI } from '../../../../util/vs/base/common/uri'; import { SyncDescriptor } from '../../../../util/vs/platform/instantiation/common/descriptors'; +import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation'; +import { MarkdownString } from '../../../../vscodeTypes'; import { createExtensionUnitTestingServices } from '../../../test/node/services'; import { ToolName } from '../../common/toolNames'; import { IToolsService } from '../../common/toolsService'; -import { IReadFileParamsV1, IReadFileParamsV2 } from '../readFileTool'; +import { IReadFileParamsV1, IReadFileParamsV2, ReadFileTool } from '../readFileTool'; import { toolResultToString } from './toolTestUtils'; suite('ReadFile', () => { @@ -24,6 +28,7 @@ suite('ReadFile', () => { const testDoc = createTextDocumentData(URI.file('/workspace/file.ts'), 'line 1\nline 2\n\nline 4\nline 5', 'ts').document; const emptyDoc = createTextDocumentData(URI.file('/workspace/empty.ts'), '', 'ts').document; const whitespaceDoc = createTextDocumentData(URI.file('/workspace/whitespace.ts'), ' \t\n', 'ts').document; + const singleLineDoc = createTextDocumentData(URI.file('/workspace/single.ts'), 'single line', 'ts').document; // Create a large document for testing truncation (3000 lines to exceed MAX_LINES_PER_READ) const largeContent = Array.from({ length: 3000 }, (_, i) => `line ${i + 1}`).join('\n'); const largeDoc = createTextDocumentData(URI.file('/workspace/large.ts'), largeContent, 'ts').document; @@ -33,7 +38,7 @@ suite('ReadFile', () => { TestWorkspaceService, [ [URI.file('/workspace')], - [testDoc, emptyDoc, whitespaceDoc, largeDoc], + [testDoc, emptyDoc, whitespaceDoc, singleLineDoc, largeDoc], ] )); accessor = services.createTestingAccessor(); @@ -242,5 +247,252 @@ suite('ReadFile', () => { expect(resultString).toContain('line 1'); expect(resultString).toContain('line 2'); }); + + test('read single-line file with offset beyond bounds should throw error with singular "line"', async () => { + const toolsService = accessor.get(IToolsService); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/single.ts', + offset: 2 + }; + await expect(toolsService.invokeTool(ToolName.ReadFile, { input, toolInvocationToken: null as never }, CancellationToken.None)) + .rejects.toThrow('Invalid offset 2: file only has 1 line. Line numbers are 1-indexed.'); + }); + + test('read file with limit of 1', async () => { + const toolsService = accessor.get(IToolsService); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/file.ts', + offset: 2, + limit: 1 + }; + const result = await toolsService.invokeTool(ToolName.ReadFile, { input, toolInvocationToken: null as never }, CancellationToken.None); + const resultString = await toolResultToString(accessor, result); + expect(resultString).toContain('line 2'); + expect(resultString).not.toContain('line 3'); + }); + }); + + suite('prepareInvocation', () => { + test('should return "Reading/Read skill" message for skill files', async () => { + const testDoc = createTextDocumentData(URI.file('/workspace/test.skill.md'), 'skill content', 'markdown').document; + + const services = createExtensionUnitTestingServices(); + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [ + [URI.file('/workspace')], + [testDoc], + ] + )); + + const mockCustomInstructions = new MockCustomInstructionsService(); + mockCustomInstructions.setSkillFiles([URI.file('/workspace/test.skill.md')]); + services.define(ICustomInstructionsService, mockCustomInstructions); + + const testAccessor = services.createTestingAccessor(); + const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/test.skill.md' + }; + + const result = await readFileTool.prepareInvocation( + { input }, + CancellationToken.None + ); + + expect(result).toBeDefined(); + expect((result!.invocationMessage as MarkdownString).value).toBe('Reading skill [workspace](file:///workspace/test.skill.md?vscodeLinkType%3Dskill)'); + expect((result!.pastTenseMessage as MarkdownString).value).toBe('Read skill [workspace](file:///workspace/test.skill.md?vscodeLinkType%3Dskill)'); + + testAccessor.dispose(); + }); + + test('should return "Reading/Read" message for non-skill files', async () => { + const testDoc = createTextDocumentData(URI.file('/workspace/test.ts'), 'code content', 'typescript').document; + + const services = createExtensionUnitTestingServices(); + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [ + [URI.file('/workspace')], + [testDoc], + ] + )); + + const mockCustomInstructions = new MockCustomInstructionsService(); + // Don't mark this file as a skill file + services.define(ICustomInstructionsService, mockCustomInstructions); + + const testAccessor = services.createTestingAccessor(); + const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/test.ts' + }; + + const result = await readFileTool.prepareInvocation( + { input }, + CancellationToken.None + ); + + expect(result).toBeDefined(); + expect((result!.invocationMessage as MarkdownString).value).toBe('Reading [](file:///workspace/test.ts)'); + expect((result!.pastTenseMessage as MarkdownString).value).toBe('Read [](file:///workspace/test.ts)'); + + testAccessor.dispose(); + }); + + test('should return "Reading skill/Read skill" message for skill files with line range', async () => { + const testDoc = createTextDocumentData(URI.file('/workspace/test.skill.md'), 'line 1\nline 2\nline 3\nline 4\nline 5', 'markdown').document; + + const services = createExtensionUnitTestingServices(); + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [ + [URI.file('/workspace')], + [testDoc], + ] + )); + + const mockCustomInstructions = new MockCustomInstructionsService(); + mockCustomInstructions.setSkillFiles([URI.file('/workspace/test.skill.md')]); + services.define(ICustomInstructionsService, mockCustomInstructions); + + const testAccessor = services.createTestingAccessor(); + const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/test.skill.md', + offset: 2, + limit: 2 + }; + + const result = await readFileTool.prepareInvocation( + { input }, + CancellationToken.None + ); + + expect(result).toBeDefined(); + // When reading a partial range of a skill file, it should say "Reading skill" + expect((result!.invocationMessage as MarkdownString).value).toBe('Reading skill [workspace](file:///workspace/test.skill.md?vscodeLinkType%3Dskill#2-2), lines 2 to 4'); + expect((result!.pastTenseMessage as MarkdownString).value).toBe('Read skill [workspace](file:///workspace/test.skill.md?vscodeLinkType%3Dskill#2-2), lines 2 to 4'); + + testAccessor.dispose(); + }); + + test('should return "Reading/Read skill" message for non-.md skill files', async () => { + const testDoc = createTextDocumentData(URI.file('/workspace/test.skill'), 'skill content', 'plaintext').document; + + const services = createExtensionUnitTestingServices(); + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [ + [URI.file('/workspace')], + [testDoc], + ] + )); + + const mockCustomInstructions = new MockCustomInstructionsService(); + mockCustomInstructions.setSkillFiles([URI.file('/workspace/test.skill')]); + services.define(ICustomInstructionsService, mockCustomInstructions); + + const testAccessor = services.createTestingAccessor(); + const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/test.skill' + }; + + const result = await readFileTool.prepareInvocation( + { input }, + CancellationToken.None + ); + + expect(result).toBeDefined(); + // For non-.md skill files, skill name should be in backticks + expect((result!.invocationMessage as MarkdownString).value).toContain('Reading skill `workspace`: [](file:///workspace/test.skill)'); + expect((result!.pastTenseMessage as MarkdownString).value).toContain('Read skill `workspace`: [](file:///workspace/test.skill)'); + + testAccessor.dispose(); + }); + + test('should return "Reading/Read skill" message for non-.md skill files with line range', async () => { + const testDoc = createTextDocumentData(URI.file('/workspace/test.skill'), 'line 1\nline 2\nline 3\nline 4\nline 5', 'plaintext').document; + + const services = createExtensionUnitTestingServices(); + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [ + [URI.file('/workspace')], + [testDoc], + ] + )); + + const mockCustomInstructions = new MockCustomInstructionsService(); + mockCustomInstructions.setSkillFiles([URI.file('/workspace/test.skill')]); + services.define(ICustomInstructionsService, mockCustomInstructions); + + const testAccessor = services.createTestingAccessor(); + const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/test.skill', + offset: 2, + limit: 2 + }; + + const result = await readFileTool.prepareInvocation( + { input }, + CancellationToken.None + ); + + expect(result).toBeDefined(); + // For non-.md skill files with range, skill name should be in backticks + expect((result!.invocationMessage as MarkdownString).value).toContain('Reading skill `workspace`: [](file:///workspace/test.skill#2-2), lines 2 to 4'); + expect((result!.pastTenseMessage as MarkdownString).value).toContain('Read skill `workspace`: [](file:///workspace/test.skill#2-2), lines 2 to 4'); + + testAccessor.dispose(); + }); + + test('should return "Reading/Read" message for non-skill files with line range', async () => { + const testDoc = createTextDocumentData(URI.file('/workspace/test.ts'), 'line 1\nline 2\nline 3\nline 4\nline 5', 'typescript').document; + + const services = createExtensionUnitTestingServices(); + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [ + [URI.file('/workspace')], + [testDoc], + ] + )); + + const mockCustomInstructions = new MockCustomInstructionsService(); + // Don't mark this file as a skill file + services.define(ICustomInstructionsService, mockCustomInstructions); + + const testAccessor = services.createTestingAccessor(); + const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool); + + const input: IReadFileParamsV2 = { + filePath: '/workspace/test.ts', + offset: 2, + limit: 2 + }; + + const result = await readFileTool.prepareInvocation( + { input }, + CancellationToken.None + ); + + expect(result).toBeDefined(); + // When reading a partial range of a non-skill file, it should say "Reading" + expect((result!.invocationMessage as MarkdownString).value).toBe('Reading [](file:///workspace/test.ts#2-2), lines 2 to 4'); + expect((result!.pastTenseMessage as MarkdownString).value).toBe('Read [](file:///workspace/test.ts#2-2), lines 2 to 4'); + + testAccessor.dispose(); + }); }); }); diff --git a/src/platform/customInstructions/common/customInstructionsService.ts b/src/platform/customInstructions/common/customInstructionsService.ts index cee481f394..ee873bab14 100644 --- a/src/platform/customInstructions/common/customInstructionsService.ts +++ b/src/platform/customInstructions/common/customInstructionsService.ts @@ -56,6 +56,8 @@ export interface ICustomInstructionsService { isExternalInstructionsFile(uri: URI): boolean; isExternalInstructionsFolder(uri: URI): boolean; isSkillFile(uri: URI): boolean; + isSkillMdFile(uri: URI): boolean; + getSkillInfo(uri: URI): { skillName: string; skillFolderUri: URI } | undefined; } export type CodeGenerationInstruction = { languagee?: string; text: string } | { languagee?: string; file: string }; @@ -90,7 +92,7 @@ export class CustomInstructionsService extends Disposable implements ICustomInst readonly _matchInstructionLocationsFromConfig: IObservable<(uri: URI) => boolean>; readonly _matchInstructionLocationsFromExtensions: IObservable<(uri: URI) => boolean>; - readonly _matchInstructionLocationsFromSkills: IObservable<(uri: URI) => boolean>; + readonly _matchInstructionLocationsFromSkills: IObservable<(uri: URI) => { skillName: string; skillFolderUri: URI } | undefined>; constructor( @IConfigurationService private readonly configurationService: IConfigurationService, @@ -165,23 +167,45 @@ export class CustomInstructionsService extends Disposable implements ICustomInst ); this._matchInstructionLocationsFromSkills = observableFromEvent( - (handleChange) => this._register(configurationService.onDidChangeConfiguration(e => { - if (e.affectsConfiguration(USE_AGENT_SKILLS_SETTING)) { - handleChange(e); - } - })), + (handleChange) => { + const configurationDisposable = configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(USE_AGENT_SKILLS_SETTING)) { + handleChange(e); + } + }); + const workspaceDisposable = workspaceService.onDidChangeWorkspaceFolders(handleChange); + return { + dispose: () => { + configurationDisposable.dispose(); + workspaceDisposable.dispose(); + } + }; + }, () => { if (this.configurationService.getNonExtensionConfig(USE_AGENT_SKILLS_SETTING)) { const personalSkillFolderUris = PERSONAL_SKILL_FOLDERS.map(folder => extUriBiasedIgnorePathCase.joinPath(this.envService.userHome, folder)); const workspaceSkillFolderUris = this.workspaceService.getWorkspaceFolders().flatMap(workspaceFolder => WORKSPACE_SKILL_FOLDERS.map(folder => extUriBiasedIgnorePathCase.joinPath(workspaceFolder, folder)) ); - const skillFolderUris = [...personalSkillFolderUris, ...workspaceSkillFolderUris]; + // List of **/skills folder URIs + const topLevelSkillsFolderUris = [...personalSkillFolderUris, ...workspaceSkillFolderUris]; return ((uri: URI) => { - return skillFolderUris.some(skillFolderUri => extUriBiasedIgnorePathCase.isEqualOrParent(uri, skillFolderUri)); + for (const topLevelSkillFolderUri of topLevelSkillsFolderUris) { + if (extUriBiasedIgnorePathCase.isEqualOrParent(uri, topLevelSkillFolderUri)) { + // Get the path segments relative to the skill folder + const relativePath = extUriBiasedIgnorePathCase.relativePath(topLevelSkillFolderUri, uri); + if (relativePath) { + // The skill directory is the first path segment under the skill folder + const skillName = relativePath.split('/')[0]; + const skillFolderUri = extUriBiasedIgnorePathCase.joinPath(topLevelSkillFolderUri, skillName); + return { skillName, skillFolderUri }; + } + } + } + return undefined; }); } - return (() => false); + return (() => undefined); } ); } @@ -285,15 +309,39 @@ export class CustomInstructionsService extends Disposable implements ICustomInst } return this._matchInstructionLocationsFromConfig.get()(uri) || this._matchInstructionLocationsFromExtensions.get()(uri) - || this._matchInstructionLocationsFromSkills.get()(uri); + || this._matchInstructionLocationsFromSkills.get()(uri) !== undefined; } public isExternalInstructionsFolder(uri: URI): boolean { return this._matchInstructionLocationsFromExtensions.get()(uri) - || this._matchInstructionLocationsFromSkills.get()(uri); + || this._matchInstructionLocationsFromSkills.get()(uri) !== undefined; } public isSkillFile(uri: URI): boolean { + return this._matchInstructionLocationsFromSkills.get()(uri) !== undefined; + } + + public isSkillMdFile(uri: URI): boolean { + return this.isSkillFile(uri) && extUriBiasedIgnorePathCase.basename(uri).toLowerCase() === 'skill.md'; + } + + public getSkillDirectory(uri: URI): URI | undefined { + const skillInfo = this._matchInstructionLocationsFromSkills.get()(uri); + if (!skillInfo) { + return undefined; + } + return skillInfo.skillFolderUri; + } + + public getSkillName(uri: URI): string | undefined { + const skillInfo = this._matchInstructionLocationsFromSkills.get()(uri); + if (!skillInfo) { + return undefined; + } + return skillInfo.skillName; + } + + public getSkillInfo(uri: URI): { skillName: string; skillFolderUri: URI } | undefined { return this._matchInstructionLocationsFromSkills.get()(uri); } } diff --git a/src/platform/customInstructions/test/node/customInstructionsService.spec.ts b/src/platform/customInstructions/test/node/customInstructionsService.spec.ts new file mode 100644 index 0000000000..5dde21c8bb --- /dev/null +++ b/src/platform/customInstructions/test/node/customInstructionsService.spec.ts @@ -0,0 +1,197 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { afterEach, beforeEach, expect, suite, test } from 'vitest'; +import { URI } from '../../../../util/vs/base/common/uri'; +import { SyncDescriptor } from '../../../../util/vs/platform/instantiation/common/descriptors'; +import { IConfigurationService } from '../../../configuration/common/configurationService'; +import { DefaultsOnlyConfigurationService } from '../../../configuration/common/defaultsOnlyConfigurationService'; +import { InMemoryConfigurationService } from '../../../configuration/test/common/inMemoryConfigurationService'; +import { createPlatformServices, ITestingServicesAccessor } from '../../../test/node/services'; +import { TestWorkspaceService } from '../../../test/node/testWorkspaceService'; +import { IWorkspaceService } from '../../../workspace/common/workspaceService'; +import { ICustomInstructionsService } from '../../common/customInstructionsService'; + +suite('CustomInstructionsService - Skills', () => { + let accessor: ITestingServicesAccessor; + let customInstructionsService: ICustomInstructionsService; + let configService: InMemoryConfigurationService; + + beforeEach(async () => { + const services = createPlatformServices(); + + // Setup workspace with a workspace folder + const workspaceFolders = [URI.file('/workspace')]; + services.define(IWorkspaceService, new SyncDescriptor( + TestWorkspaceService, + [workspaceFolders, []] + )); + + // Create a configuration service that allows setting values + configService = new InMemoryConfigurationService(new DefaultsOnlyConfigurationService()); + services.define(IConfigurationService, configService); + + // Enable the agent skills setting + await configService.setNonExtensionConfig('chat.useAgentSkills', true); + + accessor = services.createTestingAccessor(); + customInstructionsService = accessor.get(ICustomInstructionsService); + }); + + afterEach(() => { + accessor?.dispose(); + }); + + suite('getSkillInfo', () => { + test('should return skill info for file in .github/skills folder', () => { + const skillFileUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + const skillInfo = customInstructionsService.getSkillInfo(skillFileUri); + + expect(skillInfo).toBeDefined(); + expect(skillInfo?.skillName).toBe('myskill'); + expect(skillInfo?.skillFolderUri.toString()).toBe(URI.file('/workspace/.github/skills/myskill').toString()); + }); + + test('should return skill info for file in .claude/skills folder', () => { + const skillFileUri = URI.file('/workspace/.claude/skills/myskill/SKILL.md'); + const skillInfo = customInstructionsService.getSkillInfo(skillFileUri); + + expect(skillInfo).toBeDefined(); + expect(skillInfo?.skillName).toBe('myskill'); + expect(skillInfo?.skillFolderUri.toString()).toBe(URI.file('/workspace/.claude/skills/myskill').toString()); + }); + + test('should return skill info for nested file in skill folder', () => { + const skillFileUri = URI.file('/workspace/.github/skills/myskill/subfolder/helper.ts'); + const skillInfo = customInstructionsService.getSkillInfo(skillFileUri); + + expect(skillInfo).toBeDefined(); + expect(skillInfo?.skillName).toBe('myskill'); + expect(skillInfo?.skillFolderUri.toString()).toBe(URI.file('/workspace/.github/skills/myskill').toString()); + }); + + test('should return undefined for non-skill file', () => { + const regularFileUri = URI.file('/workspace/src/file.ts'); + const skillInfo = customInstructionsService.getSkillInfo(regularFileUri); + + expect(skillInfo).toBeUndefined(); + }); + + test('should return undefined when useAgentSkills setting is disabled', async () => { + // Disable the setting + await configService.setNonExtensionConfig('chat.useAgentSkills', false); + + const skillFileUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + const skillInfo = customInstructionsService.getSkillInfo(skillFileUri); + + expect(skillInfo).toBeUndefined(); + }); + + test('should return skill info for skill with hyphenated name', () => { + const skillFileUri = URI.file('/workspace/.github/skills/my-skill-name/SKILL.md'); + const skillInfo = customInstructionsService.getSkillInfo(skillFileUri); + + expect(skillInfo).toBeDefined(); + expect(skillInfo?.skillName).toBe('my-skill-name'); + }); + }); + + suite('isSkillFile', () => { + test('should return true for file in skill folder', () => { + const skillFileUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + expect(customInstructionsService.isSkillFile(skillFileUri)).toBe(true); + }); + + test('should return true for nested file in skill folder', () => { + const skillFileUri = URI.file('/workspace/.github/skills/myskill/subfolder/code.ts'); + expect(customInstructionsService.isSkillFile(skillFileUri)).toBe(true); + }); + + test('should return false for non-skill file', () => { + const regularFileUri = URI.file('/workspace/src/file.ts'); + expect(customInstructionsService.isSkillFile(regularFileUri)).toBe(false); + }); + + test('should return false when useAgentSkills setting is disabled', async () => { + await configService.setNonExtensionConfig('chat.useAgentSkills', false); + + const skillFileUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + expect(customInstructionsService.isSkillFile(skillFileUri)).toBe(false); + }); + + test('should return true for file in .claude/skills folder', () => { + const skillFileUri = URI.file('/workspace/.claude/skills/test/file.ts'); + expect(customInstructionsService.isSkillFile(skillFileUri)).toBe(true); + }); + }); + + suite('isSkillMdFile', () => { + test('should return true for SKILL.md in skill folder', () => { + const skillMdUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + expect(customInstructionsService.isSkillMdFile(skillMdUri)).toBe(true); + }); + + test('should return true for skill.md with lowercase', () => { + const skillMdUri = URI.file('/workspace/.github/skills/myskill/skill.md'); + expect(customInstructionsService.isSkillMdFile(skillMdUri)).toBe(true); + }); + + test('should return true for mixed case sKiLl.Md', () => { + const skillMdUri = URI.file('/workspace/.github/skills/myskill/sKiLl.Md'); + expect(customInstructionsService.isSkillMdFile(skillMdUri)).toBe(true); + }); + + test('should return false for other .md files in skill folder', () => { + const otherMdUri = URI.file('/workspace/.github/skills/myskill/README.md'); + expect(customInstructionsService.isSkillMdFile(otherMdUri)).toBe(false); + }); + + test('should return false for non-md files in skill folder', () => { + const codeFileUri = URI.file('/workspace/.github/skills/myskill/code.ts'); + expect(customInstructionsService.isSkillMdFile(codeFileUri)).toBe(false); + }); + + test('should return false for SKILL.md outside skill folder', () => { + const nonSkillUri = URI.file('/workspace/docs/SKILL.md'); + expect(customInstructionsService.isSkillMdFile(nonSkillUri)).toBe(false); + }); + + test('should return false when useAgentSkills setting is disabled', async () => { + await configService.setNonExtensionConfig('chat.useAgentSkills', false); + + const skillMdUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + expect(customInstructionsService.isSkillMdFile(skillMdUri)).toBe(false); + }); + }); + + suite('isExternalInstructionsFile', () => { + test('should return true for skill files', () => { + const skillFileUri = URI.file('/workspace/.github/skills/myskill/SKILL.md'); + expect(customInstructionsService.isExternalInstructionsFile(skillFileUri)).toBe(true); + }); + + test('should return false for regular files', () => { + const regularFileUri = URI.file('/workspace/src/file.ts'); + expect(customInstructionsService.isExternalInstructionsFile(regularFileUri)).toBe(false); + }); + }); + + suite('isExternalInstructionsFolder', () => { + test('should return true for skill folder', () => { + const skillFolderUri = URI.file('/workspace/.github/skills/myskill'); + expect(customInstructionsService.isExternalInstructionsFolder(skillFolderUri)).toBe(true); + }); + + test('should return true for nested folder in skill', () => { + const nestedFolderUri = URI.file('/workspace/.github/skills/myskill/subfolder'); + expect(customInstructionsService.isExternalInstructionsFolder(nestedFolderUri)).toBe(true); + }); + + test('should return false for regular folder', () => { + const regularFolderUri = URI.file('/workspace/src'); + expect(customInstructionsService.isExternalInstructionsFolder(regularFolderUri)).toBe(false); + }); + }); +}); diff --git a/src/platform/test/common/testCustomInstructionsService.ts b/src/platform/test/common/testCustomInstructionsService.ts new file mode 100644 index 0000000000..3bff1adcf4 --- /dev/null +++ b/src/platform/test/common/testCustomInstructionsService.ts @@ -0,0 +1,101 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { URI } from '../../../util/vs/base/common/uri'; +import type { Uri } from '../../../vscodeTypes'; +import { Config } from '../../configuration/common/configurationService'; +import { CodeGenerationInstruction, ICustomInstructions, ICustomInstructionsService } from '../../customInstructions/common/customInstructionsService'; + +/** + * A configurable mock implementation of ICustomInstructionsService for testing. + * Allows setting skill files and external instruction files for different test scenarios. + */ +export class MockCustomInstructionsService implements ICustomInstructionsService { + declare readonly _serviceBrand: undefined; + + private skillFiles = new Set(); + private externalFiles = new Set(); + private externalFolders = new Set(); + + /** + * Set the URIs that should be recognized as skill files. + */ + setSkillFiles(uris: URI[]): void { + this.skillFiles.clear(); + uris.forEach(uri => this.skillFiles.add(uri.toString())); + } + + /** + * Set the URIs that should be recognized as external instruction files. + */ + setExternalFiles(uris: URI[]): void { + this.externalFiles.clear(); + uris.forEach(uri => this.externalFiles.add(uri.toString())); + } + + /** + * Set the URIs that should be recognized as external instruction folders. + */ + setExternalFolders(uris: URI[]): void { + this.externalFolders.clear(); + uris.forEach(uri => this.externalFolders.add(uri.toString())); + } + + isSkillFile(uri: URI): boolean { + return this.skillFiles.has(uri.toString()); + } + + isSkillMdFile(uri: URI): boolean { + return this.isSkillFile(uri) && uri.path.toLowerCase().endsWith('skill.md'); + } + + getSkillDirectory(uri: URI): URI { + // Simple mock implementation: return parent directory + return URI.parse(uri.toString().substring(0, uri.toString().lastIndexOf('/'))); + } + + getSkillName(uri: URI): string { + const skillDir = this.getSkillDirectory(uri); + const path = skillDir.path; + return path.substring(path.lastIndexOf('/') + 1); + } + + getSkillMdUri(uri: URI): URI { + if (this.isSkillMdFile(uri)) { + return uri; + } + const skillDir = this.getSkillDirectory(uri); + return URI.joinPath(skillDir, 'SKILL.md'); + } + + getSkillInfo(uri: URI): { skillName: string; skillFolderUri: URI } | undefined { + if (!this.isSkillFile(uri)) { + return undefined; + } + const skillFolderUri = this.getSkillDirectory(uri); + const skillName = this.getSkillName(uri); + return { skillName, skillFolderUri }; + } + + isExternalInstructionsFile(uri: URI): boolean { + return this.externalFiles.has(uri.toString()); + } + + isExternalInstructionsFolder(uri: URI): boolean { + return this.externalFolders.has(uri.toString()); + } + + fetchInstructionsFromSetting(_configKey: Config): Promise { + return Promise.resolve([]); + } + + fetchInstructionsFromFile(_fileUri: Uri): Promise { + return Promise.resolve(undefined); + } + + getAgentInstructions(): Promise { + return Promise.resolve([]); + } +}