mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-01 21:20:44 +00:00
Merge pull request #2202 from QwenLM/feature/support-agents-directory-skills
feat: support skills in .agents directory and other provider directories
This commit is contained in:
commit
fda4e85503
11 changed files with 197 additions and 80 deletions
|
|
@ -12,6 +12,7 @@ import { getProjectHash, sanitizeCwd } from '../utils/paths.js';
|
|||
export const QWEN_DIR = '.qwen';
|
||||
export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json';
|
||||
export const OAUTH_FILE = 'oauth_creds.json';
|
||||
export const SKILL_PROVIDER_CONFIG_DIRS = ['.qwen', '.agent'];
|
||||
const TMP_DIR_NAME = 'tmp';
|
||||
const BIN_DIR_NAME = 'bin';
|
||||
const PROJECT_DIR_NAME = 'projects';
|
||||
|
|
@ -138,8 +139,11 @@ export class Storage {
|
|||
return path.join(this.getExtensionsDir(), 'qwen-extension.json');
|
||||
}
|
||||
|
||||
getUserSkillsDir(): string {
|
||||
return path.join(Storage.getGlobalQwenDir(), 'skills');
|
||||
getUserSkillsDirs(): string[] {
|
||||
const homeDir = os.homedir() || os.tmpdir();
|
||||
return SKILL_PROVIDER_CONFIG_DIRS.map((dir) =>
|
||||
path.join(homeDir, dir, 'skills'),
|
||||
);
|
||||
}
|
||||
|
||||
getHistoryFilePath(): string {
|
||||
|
|
|
|||
|
|
@ -57,14 +57,10 @@ function getWindowsPathFingerprint(
|
|||
env: NodeJS.ProcessEnv,
|
||||
pathKeys: string[],
|
||||
): string {
|
||||
return pathKeys
|
||||
.map((key) => `${key}=${env[key] ?? ''}`)
|
||||
.join('\0');
|
||||
return pathKeys.map((key) => `${key}=${env[key] ?? ''}`).join('\0');
|
||||
}
|
||||
|
||||
function normalizePathEnvForWindows(
|
||||
env: NodeJS.ProcessEnv,
|
||||
): NodeJS.ProcessEnv {
|
||||
function normalizePathEnvForWindows(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
|
||||
if (os.platform() !== 'win32') {
|
||||
return env;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -73,6 +73,14 @@ describe('SkillManager', () => {
|
|||
if (yamlString.includes('name: regular-skill')) {
|
||||
return { name: 'regular-skill', description: 'A regular skill' };
|
||||
}
|
||||
if (yamlString.includes('name: shared-skill')) {
|
||||
const desc = yamlString.includes('From qwen dir')
|
||||
? 'From qwen dir'
|
||||
: yamlString.includes('From agent dir')
|
||||
? 'From agent dir'
|
||||
: 'A shared skill';
|
||||
return { name: 'shared-skill', description: desc };
|
||||
}
|
||||
if (!yamlString.includes('name:')) {
|
||||
return { description: 'A test skill' }; // Missing name case
|
||||
}
|
||||
|
|
@ -391,42 +399,61 @@ You are a helpful assistant.
|
|||
|
||||
describe('listSkills', () => {
|
||||
beforeEach(() => {
|
||||
// Mock directory listing for skills directories (with Dirent objects)
|
||||
vi.mocked(fs.readdir)
|
||||
.mockResolvedValueOnce([
|
||||
{
|
||||
name: 'skill1',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
{
|
||||
name: 'skill2',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
{
|
||||
name: 'not-a-dir.txt',
|
||||
isDirectory: () => false,
|
||||
isFile: () => true,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
] as unknown as Awaited<ReturnType<typeof fs.readdir>>)
|
||||
.mockResolvedValueOnce([
|
||||
{
|
||||
name: 'skill3',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
{
|
||||
name: 'skill1',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
|
||||
// Mock directory listing based on path to handle multiple base dirs per level.
|
||||
// Use path.join to construct expected paths so separators match on all platforms.
|
||||
const projectQwenSkillsDir = path.join(
|
||||
'/test/project',
|
||||
'.qwen',
|
||||
'skills',
|
||||
);
|
||||
const userQwenSkillsDir = path.join('/home/user', '.qwen', 'skills');
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
vi.mocked(fs.readdir).mockImplementation((dirPath: any) => {
|
||||
const pathStr = String(dirPath);
|
||||
if (pathStr === projectQwenSkillsDir) {
|
||||
return Promise.resolve([
|
||||
{
|
||||
name: 'skill1',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
{
|
||||
name: 'skill2',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
{
|
||||
name: 'not-a-dir.txt',
|
||||
isDirectory: () => false,
|
||||
isFile: () => true,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
|
||||
}
|
||||
if (pathStr === userQwenSkillsDir) {
|
||||
return Promise.resolve([
|
||||
{
|
||||
name: 'skill3',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
{
|
||||
name: 'skill1',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
|
||||
}
|
||||
// Other provider dirs (.agent, .cursor, .codex, .claude) return empty
|
||||
return Promise.resolve(
|
||||
[] as unknown as Awaited<ReturnType<typeof fs.readdir>>,
|
||||
);
|
||||
});
|
||||
|
||||
vi.mocked(fs.access).mockResolvedValue(undefined);
|
||||
|
||||
|
|
@ -483,6 +510,66 @@ Skill 3 content`);
|
|||
expect(projectSkills.every((s) => s.level === 'project')).toBe(true);
|
||||
});
|
||||
|
||||
it('should deduplicate same-name skills across provider dirs within a level', async () => {
|
||||
// Override readdir to return the same skill name from both .qwen and .agent dirs
|
||||
vi.mocked(fs.readdir).mockReset();
|
||||
const projectQwenDir = path.join('/test/project', '.qwen', 'skills');
|
||||
const projectAgentDir = path.join('/test/project', '.agent', 'skills');
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
vi.mocked(fs.readdir).mockImplementation((dirPath: any) => {
|
||||
const pathStr = String(dirPath);
|
||||
if (pathStr === projectQwenDir) {
|
||||
return Promise.resolve([
|
||||
{
|
||||
name: 'shared-skill',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
|
||||
}
|
||||
if (pathStr === projectAgentDir) {
|
||||
return Promise.resolve([
|
||||
{
|
||||
name: 'shared-skill',
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
},
|
||||
] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
|
||||
}
|
||||
return Promise.resolve(
|
||||
[] as unknown as Awaited<ReturnType<typeof fs.readdir>>,
|
||||
);
|
||||
});
|
||||
|
||||
vi.mocked(fs.readFile).mockImplementation((filePath) => {
|
||||
const pathStr = String(filePath);
|
||||
if (pathStr.includes('.qwen') && pathStr.includes('shared-skill')) {
|
||||
return Promise.resolve(
|
||||
`---\nname: shared-skill\ndescription: From qwen dir\n---\nQwen content`,
|
||||
);
|
||||
}
|
||||
if (pathStr.includes('.agent') && pathStr.includes('shared-skill')) {
|
||||
return Promise.resolve(
|
||||
`---\nname: shared-skill\ndescription: From agent dir\n---\nAgent content`,
|
||||
);
|
||||
}
|
||||
return Promise.reject(new Error('File not found'));
|
||||
});
|
||||
|
||||
const skills = await manager.listSkills({
|
||||
level: 'project',
|
||||
force: true,
|
||||
});
|
||||
|
||||
// Only one instance should remain, from .qwen (first in PROVIDER_CONFIG_DIRS)
|
||||
expect(skills).toHaveLength(1);
|
||||
expect(skills[0].name).toBe('shared-skill');
|
||||
expect(skills[0].description).toBe('From qwen dir');
|
||||
});
|
||||
|
||||
it('should handle empty directories', async () => {
|
||||
vi.mocked(fs.readdir).mockReset();
|
||||
vi.mocked(fs.readdir).mockResolvedValue(
|
||||
|
|
@ -504,27 +591,33 @@ Skill 3 content`);
|
|||
});
|
||||
});
|
||||
|
||||
describe('getSkillsBaseDir', () => {
|
||||
it('should return project-level base dir', () => {
|
||||
const baseDir = manager.getSkillsBaseDir('project');
|
||||
describe('getSkillsBaseDirs', () => {
|
||||
it('should return all project-level base dirs', () => {
|
||||
const baseDirs = manager.getSkillsBaseDirs('project');
|
||||
|
||||
expect(baseDir).toBe(path.join('/test/project', '.qwen', 'skills'));
|
||||
expect(baseDirs).toHaveLength(2);
|
||||
expect(baseDirs).toContain(path.join('/test/project', '.qwen', 'skills'));
|
||||
expect(baseDirs).toContain(
|
||||
path.join('/test/project', '.agent', 'skills'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should return user-level base dir', () => {
|
||||
const baseDir = manager.getSkillsBaseDir('user');
|
||||
it('should return all user-level base dirs', () => {
|
||||
const baseDirs = manager.getSkillsBaseDirs('user');
|
||||
|
||||
expect(baseDir).toBe(path.join('/home/user', '.qwen', 'skills'));
|
||||
expect(baseDirs).toHaveLength(2);
|
||||
expect(baseDirs).toContain(path.join('/home/user', '.qwen', 'skills'));
|
||||
expect(baseDirs).toContain(path.join('/home/user', '.agent', 'skills'));
|
||||
});
|
||||
|
||||
it('should return bundled-level base dir', () => {
|
||||
const baseDir = manager.getSkillsBaseDir('bundled');
|
||||
const baseDirs = manager.getSkillsBaseDirs('bundled');
|
||||
|
||||
expect(baseDir).toMatch(/skills[/\\]bundled$/);
|
||||
expect(baseDirs[0]).toMatch(/skills[/\\]bundled$/);
|
||||
});
|
||||
|
||||
it('should throw for extension level', () => {
|
||||
expect(() => manager.getSkillsBaseDir('extension')).toThrow(
|
||||
expect(() => manager.getSkillsBaseDirs('extension')).toThrow(
|
||||
'Extension skills do not have a base directory',
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import type { Config } from '../config/config.js';
|
|||
import { validateConfig } from './skill-load.js';
|
||||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import { normalizeContent } from '../utils/textUtils.js';
|
||||
import { SKILL_PROVIDER_CONFIG_DIRS } from '../config/storage.js';
|
||||
|
||||
const debugLogger = createDebugLogger('SKILL_MANAGER');
|
||||
|
||||
|
|
@ -428,20 +429,20 @@ export class SkillManager {
|
|||
* Gets the base directory for skills at a specific level.
|
||||
*
|
||||
* @param level - Storage level
|
||||
* @returns Absolute directory path
|
||||
* @returns Absolute directory paths
|
||||
*/
|
||||
getSkillsBaseDir(level: SkillLevel): string {
|
||||
getSkillsBaseDirs(level: SkillLevel): string[] {
|
||||
switch (level) {
|
||||
case 'project':
|
||||
return path.join(
|
||||
this.config.getProjectRoot(),
|
||||
QWEN_CONFIG_DIR,
|
||||
SKILLS_CONFIG_DIR,
|
||||
return SKILL_PROVIDER_CONFIG_DIRS.map((v) =>
|
||||
path.join(this.config.getProjectRoot(), v, SKILLS_CONFIG_DIR),
|
||||
);
|
||||
case 'user':
|
||||
return path.join(os.homedir(), QWEN_CONFIG_DIR, SKILLS_CONFIG_DIR);
|
||||
return SKILL_PROVIDER_CONFIG_DIRS.map((v) =>
|
||||
path.join(os.homedir(), v, SKILLS_CONFIG_DIR),
|
||||
);
|
||||
case 'bundled':
|
||||
return this.bundledSkillsDir;
|
||||
return [this.bundledSkillsDir];
|
||||
case 'extension':
|
||||
throw new Error(
|
||||
'Extension skills do not have a base directory; they are loaded from active extensions.',
|
||||
|
|
@ -499,9 +500,26 @@ export class SkillManager {
|
|||
return skills;
|
||||
}
|
||||
|
||||
const baseDir = this.getSkillsBaseDir(level);
|
||||
debugLogger.debug(`Loading ${level} level skills from: ${baseDir}`);
|
||||
const skills = await this.loadSkillsFromDir(baseDir, level);
|
||||
// Iterate provider directories in PROVIDER_CONFIG_DIRS order.
|
||||
// The first directory that contains a skill with a given name wins,
|
||||
// so the order defines implicit precedence (.qwen > .agent > .cursor > ...).
|
||||
const baseDirs = this.getSkillsBaseDirs(level);
|
||||
const skills: SkillConfig[] = [];
|
||||
const seenNames = new Set<string>();
|
||||
for (const baseDir of baseDirs) {
|
||||
debugLogger.debug(`Loading ${level} level skills from: ${baseDir}`);
|
||||
const skillsFromDir = await this.loadSkillsFromDir(baseDir, level);
|
||||
for (const skill of skillsFromDir) {
|
||||
if (seenNames.has(skill.name)) {
|
||||
debugLogger.debug(
|
||||
`Skipping duplicate skill at ${level} level: ${skill.name} from ${baseDir}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
seenNames.add(skill.name);
|
||||
skills.push(skill);
|
||||
}
|
||||
}
|
||||
debugLogger.debug(`Loaded ${skills.length} ${level} level skills`);
|
||||
return skills;
|
||||
}
|
||||
|
|
@ -624,7 +642,8 @@ export class SkillManager {
|
|||
private updateWatchersFromCache(): void {
|
||||
const watchTargets = new Set<string>(
|
||||
(['project', 'user'] as const)
|
||||
.map((level) => this.getSkillsBaseDir(level))
|
||||
.map((level) => this.getSkillsBaseDirs(level))
|
||||
.reduce((acc, baseDirs) => acc.concat(baseDirs), [])
|
||||
.filter((baseDir) => fsSync.existsSync(baseDir)),
|
||||
);
|
||||
|
||||
|
|
@ -680,7 +699,7 @@ export class SkillManager {
|
|||
}
|
||||
|
||||
private async ensureUserSkillsDir(): Promise<void> {
|
||||
const baseDir = this.getSkillsBaseDir('user');
|
||||
const baseDir = path.join(os.homedir(), QWEN_CONFIG_DIR, SKILLS_CONFIG_DIR);
|
||||
try {
|
||||
await fs.mkdir(baseDir, { recursive: true });
|
||||
} catch (error) {
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ describe('LSTool', () => {
|
|||
}),
|
||||
getTruncateToolOutputLines: () => 1000,
|
||||
storage: {
|
||||
getUserSkillsDir: () => userSkillsBase,
|
||||
getUserSkillsDirs: () => [userSkillsBase],
|
||||
},
|
||||
} as unknown as Config;
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import path from 'node:path';
|
|||
import type { ToolInvocation, ToolResult } from './tools.js';
|
||||
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
|
||||
import { makeRelative, shortenPath } from '../utils/paths.js';
|
||||
import { isSubpath } from '../utils/paths.js';
|
||||
import { isSubpaths } from '../utils/paths.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
|
@ -335,8 +335,8 @@ export class LSTool extends BaseDeclarativeTool<LSToolParams, ToolResult> {
|
|||
return `Path must be absolute: ${params.path}`;
|
||||
}
|
||||
|
||||
const userSkillsBase = this.config.storage.getUserSkillsDir();
|
||||
const isUnderUserSkills = isSubpath(userSkillsBase, params.path);
|
||||
const userSkillsBases = this.config.storage.getUserSkillsDirs();
|
||||
const isUnderUserSkills = isSubpaths(userSkillsBases, params.path);
|
||||
|
||||
const workspaceContext = this.config.getWorkspaceContext();
|
||||
if (
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ describe('ReadFileTool', () => {
|
|||
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
|
||||
storage: {
|
||||
getProjectTempDir: () => path.join(tempRootDir, '.temp'),
|
||||
getUserSkillsDir: () => path.join(os.homedir(), '.qwen', 'skills'),
|
||||
getUserSkillsDirs: () => [path.join(os.homedir(), '.qwen', 'skills')],
|
||||
},
|
||||
getTruncateToolOutputThreshold: () => 2500,
|
||||
getTruncateToolOutputLines: () => 500,
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ import { FileOperation } from '../telemetry/metrics.js';
|
|||
import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js';
|
||||
import { logFileOperation } from '../telemetry/loggers.js';
|
||||
import { FileOperationEvent } from '../telemetry/types.js';
|
||||
import { isSubpath } from '../utils/paths.js';
|
||||
import { isSubpaths, isSubpath } from '../utils/paths.js';
|
||||
import { Storage } from '../config/storage.js';
|
||||
|
||||
/**
|
||||
|
|
@ -187,7 +187,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
|
|||
const workspaceContext = this.config.getWorkspaceContext();
|
||||
const globalTempDir = Storage.getGlobalTempDir();
|
||||
const projectTempDir = this.config.storage.getProjectTempDir();
|
||||
const userSkillsDir = this.config.storage.getUserSkillsDir();
|
||||
const userSkillsDirs = this.config.storage.getUserSkillsDirs();
|
||||
const arenaDir = Storage.getGlobalArenaDir();
|
||||
const resolvedFilePath = path.resolve(filePath);
|
||||
const osTempDir = os.tmpdir();
|
||||
|
|
@ -195,8 +195,9 @@ export class ReadFileTool extends BaseDeclarativeTool<
|
|||
isSubpath(projectTempDir, resolvedFilePath) ||
|
||||
isSubpath(globalTempDir, resolvedFilePath) ||
|
||||
isSubpath(osTempDir, resolvedFilePath);
|
||||
|
||||
const isWithinUserSkills = isSubpaths(userSkillsDirs, resolvedFilePath);
|
||||
const isWithinArenaDir = isSubpath(arenaDir, resolvedFilePath);
|
||||
const isWithinUserSkills = isSubpath(userSkillsDir, resolvedFilePath);
|
||||
|
||||
if (
|
||||
!workspaceContext.isPathWithinWorkspace(filePath) &&
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ describe('ShellTool', () => {
|
|||
.fn()
|
||||
.mockReturnValue(createMockWorkspaceContext('/test/dir')),
|
||||
storage: {
|
||||
getUserSkillsDir: vi.fn().mockReturnValue('/test/dir/.qwen/skills'),
|
||||
getUserSkillsDirs: vi.fn().mockReturnValue(['/test/dir/.qwen/skills']),
|
||||
getProjectTempDir: vi.fn().mockReturnValue('/tmp/qwen-temp'),
|
||||
},
|
||||
getTruncateToolOutputThreshold: vi.fn().mockReturnValue(0),
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ import type {
|
|||
import { ShellExecutionService } from '../services/shellExecutionService.js';
|
||||
import { formatMemoryUsage } from '../utils/formatters.js';
|
||||
import type { AnsiOutput } from '../utils/terminalSerializer.js';
|
||||
import { isSubpath } from '../utils/paths.js';
|
||||
import { isSubpaths } from '../utils/paths.js';
|
||||
import {
|
||||
getCommandRoots,
|
||||
isCommandAllowed,
|
||||
|
|
@ -622,10 +622,10 @@ export class ShellTool extends BaseDeclarativeTool<
|
|||
return 'Directory must be an absolute path.';
|
||||
}
|
||||
|
||||
const userSkillsDir = this.config.storage.getUserSkillsDir();
|
||||
const userSkillsDirs = this.config.storage.getUserSkillsDirs();
|
||||
const resolvedDirectoryPath = path.resolve(params.directory);
|
||||
const isWithinUserSkills = isSubpath(
|
||||
userSkillsDir,
|
||||
const isWithinUserSkills = isSubpaths(
|
||||
userSkillsDirs,
|
||||
resolvedDirectoryPath,
|
||||
);
|
||||
if (isWithinUserSkills) {
|
||||
|
|
|
|||
|
|
@ -241,6 +241,10 @@ export function isSubpath(parentPath: string, childPath: string): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
export function isSubpaths(parentPath: string[], childPath: string): boolean {
|
||||
return parentPath.some((p) => isSubpath(p, childPath));
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a path with tilde (~) expansion and relative path resolution.
|
||||
* Handles tilde expansion for home directory and resolves relative paths
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue