From b1d9324f5d1b468d4e5ed6e3a024effc966d7f24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 2 May 2026 16:45:28 +0000 Subject: [PATCH] fix(skills): XML-escape description/whenToUse; symlinks in skill-load.ts; dot:true in ConditionalRulesRegistry Agent-Logs-Url: https://github.com/QwenLM/qwen-code/sessions/a56d83ce-cbdf-4213-a90a-888a9f05ee4f --- packages/core/src/skills/skill-load.test.ts | 98 ++++++++++++++++++- packages/core/src/skills/skill-load.ts | 26 ++++- packages/core/src/tools/skill.test.ts | 27 +++++ packages/core/src/tools/skill.ts | 17 +++- .../core/src/utils/rulesDiscovery.test.ts | 13 +++ packages/core/src/utils/rulesDiscovery.ts | 2 +- 6 files changed, 174 insertions(+), 9 deletions(-) diff --git a/packages/core/src/skills/skill-load.test.ts b/packages/core/src/skills/skill-load.test.ts index 9f7700506..58bd2549f 100644 --- a/packages/core/src/skills/skill-load.test.ts +++ b/packages/core/src/skills/skill-load.test.ts @@ -212,8 +212,18 @@ Some content without frontmatter. it('should load skills from directory', async () => { vi.mocked(fs.readdir).mockResolvedValue([ - { name: 'skill1', isDirectory: () => true, isFile: () => false }, - { name: 'not-a-dir.txt', isDirectory: () => false, isFile: () => true }, + { + name: 'skill1', + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }, + { + name: 'not-a-dir.txt', + isDirectory: () => false, + isFile: () => true, + isSymbolicLink: () => false, + }, ] as unknown as Awaited>); vi.mocked(fs.access).mockResolvedValue(undefined); @@ -241,8 +251,18 @@ Skill body. it('should skip skills with invalid YAML and continue loading others', async () => { vi.mocked(fs.readdir).mockResolvedValue([ - { name: 'valid-skill', isDirectory: () => true, isFile: () => false }, - { name: 'invalid-skill', isDirectory: () => true, isFile: () => false }, + { + name: 'valid-skill', + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }, + { + name: 'invalid-skill', + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }, ] as unknown as Awaited>); vi.mocked(fs.access).mockResolvedValue(undefined); @@ -265,6 +285,76 @@ Valid skill. expect(skills).toHaveLength(1); expect(skills[0]?.name).toBe('test-skill'); }); + + it('should load skills from symlinked directories', async () => { + vi.mocked(fs.readdir).mockResolvedValue([ + { + name: 'symlinked-skill', + isDirectory: () => false, + isFile: () => false, + isSymbolicLink: () => true, + }, + ] as unknown as Awaited>); + + // stat resolves to a directory (symlink target) + vi.mocked(fs.stat).mockResolvedValue({ + isDirectory: () => true, + } as unknown as Awaited>); + + vi.mocked(fs.access).mockResolvedValue(undefined); + vi.mocked(fs.readFile).mockResolvedValue(`--- +name: test-skill +description: A test skill +--- + +Symlinked skill body. +`); + + const skills = await loadSkillsFromDir(testBaseDir); + + // Skill is loaded from the symlinked directory. + expect(skills).toHaveLength(1); + }); + + it('should skip symlinks that do not point to a directory', async () => { + vi.mocked(fs.readdir).mockResolvedValue([ + { + name: 'file-symlink', + isDirectory: () => false, + isFile: () => false, + isSymbolicLink: () => true, + }, + ] as unknown as Awaited>); + + // stat resolves to a file (not a directory) + vi.mocked(fs.stat).mockResolvedValue({ + isDirectory: () => false, + } as unknown as Awaited>); + + const skills = await loadSkillsFromDir(testBaseDir); + + expect(skills).toHaveLength(0); + }); + + it('should skip broken symlinks gracefully', async () => { + vi.mocked(fs.readdir).mockResolvedValue([ + { + name: 'broken-symlink', + isDirectory: () => false, + isFile: () => false, + isSymbolicLink: () => true, + }, + ] as unknown as Awaited>); + + // stat throws because the symlink target doesn't exist + vi.mocked(fs.stat).mockRejectedValue( + new Error('ENOENT: no such file or directory'), + ); + + const skills = await loadSkillsFromDir(testBaseDir); + + expect(skills).toHaveLength(0); + }); }); describe('validateConfig', () => { diff --git a/packages/core/src/skills/skill-load.ts b/packages/core/src/skills/skill-load.ts index 8b031e186..c132671cb 100644 --- a/packages/core/src/skills/skill-load.ts +++ b/packages/core/src/skills/skill-load.ts @@ -25,13 +25,35 @@ export async function loadSkillsFromDir( debugLogger.debug(`Found ${entries.length} entries in ${baseDir}`); for (const entry of entries) { - // Only process directories (each skill is a directory) - if (!entry.isDirectory()) { + // Process directories and symlinks that resolve to directories. + // Plain files are silently skipped (each skill must be a directory). + const isDirectory = entry.isDirectory(); + const isSymlink = entry.isSymbolicLink(); + + if (!isDirectory && !isSymlink) { debugLogger.warn(`Skipping non-directory entry: ${entry.name}`); continue; } const skillDir = path.join(baseDir, entry.name); + + // For symlinks, verify the target resolves to a directory. + if (isSymlink) { + try { + const targetStat = await fs.stat(skillDir); + if (!targetStat.isDirectory()) { + debugLogger.warn( + `Skipping symlink ${entry.name} that does not point to a directory`, + ); + continue; + } + } catch (error) { + debugLogger.warn( + `Skipping invalid symlink ${entry.name}: ${error instanceof Error ? error.message : 'Unknown error'}`, + ); + continue; + } + } const skillManifest = path.join(skillDir, SKILL_MANIFEST_FILE); try { diff --git a/packages/core/src/tools/skill.test.ts b/packages/core/src/tools/skill.test.ts index 5a7e915df..3b8120369 100644 --- a/packages/core/src/tools/skill.test.ts +++ b/packages/core/src/tools/skill.test.ts @@ -146,6 +146,33 @@ describe('SkillTool', () => { ); }); + it('should XML-escape description and whenToUse fields', async () => { + // A crafted description containing XML-special characters must not + // inject raw tags into the block. + vi.mocked(mockSkillManager.listSkills).mockResolvedValue([ + { + name: 'xss-skill', + description: 'Skill bold & more', + whenToUse: 'When