From eaf23caf046fd82a66d376bf4cdfeaafba83f4a7 Mon Sep 17 00:00:00 2001 From: hetaoBackend Date: Wed, 23 Sep 2026 17:08:17 +0800 Subject: [PATCH] fix(skills): discover linked workspace skill directories --- docs/tui-capabilities.md | 9 + packages/agent-modules/skills/src/registry.ts | 21 +- packages/agent-modules/skills/src/types.ts | 2 + packages/local-runtime/src/skills/roots.ts | 1 + .../unit/skill-directory-symlinks.test.ts | 233 ++++++++++++++++++ release/public-source.json | 1 + test/vitest-suites.json | 1 + 7 files changed, 264 insertions(+), 4 deletions(-) create mode 100644 packages/local-runtime/test/unit/skill-directory-symlinks.test.ts diff --git a/docs/tui-capabilities.md b/docs/tui-capabilities.md index a67fb9f2..622039f2 100644 --- a/docs/tui-capabilities.md +++ b/docs/tui-capabilities.md @@ -21,6 +21,15 @@ The evidence column summarizes the historical TUI 0.3.11 restoration record from | Files, shell, subagents, sessions, headless, ACP | Actual runtime retained | BYOK, file reads, session resume, ACP, sandbox, and status protocol tests | | Built-in skills, MCP, plugin tools | Original TUI assets and activation conditions retained | Asset build, plugin, and MCP tests; no claim that every skill has passed a real task | +## Skill directory links + +Workspace `.agents/skills`, `.claude/skills`, and `.minimax/skills` support +directory symlinks, both for the entire skill root and for individual skill +directories. Targets may live outside the workspace. Existing external-source +enable settings and duplicate-name priority still apply. Linked directories are +watched for `SKILL.md` creation and edits; broken links are skipped. `SKILL.md` +itself must remain a regular file. + ## ACP Skill commands ACP clients receive enabled Skills alongside built-in slash commands when a session diff --git a/packages/agent-modules/skills/src/registry.ts b/packages/agent-modules/skills/src/registry.ts index 50c469b4..97277ce6 100644 --- a/packages/agent-modules/skills/src/registry.ts +++ b/packages/agent-modules/skills/src/registry.ts @@ -226,7 +226,7 @@ export class SkillRegistry { }; for (const root of this.roots) { collectPath(root.rootPath, root); - for (const skillDir of listWatchableSkillDirs(root.rootPath)) { + for (const skillDir of listWatchableSkillDirs(root)) { collectPath(skillDir, root); } } @@ -472,7 +472,10 @@ function existingWatchTarget(targetPath: string): SkillWatchTarget | undefined { } function allowsDirectorySymlinkOutsideRoot(root: SkillSourceRoot): boolean { - return ['agent', 'global', 'user', 'builtin'].includes(root.kind); + return ( + root.allowDirectorySymlinksOutsideRoot === true || + ['agent', 'global', 'user', 'builtin'].includes(root.kind) + ); } async function listSkillFiles( @@ -811,10 +814,20 @@ function sameSkillFileStat(expected: SkillFileStat, actual: SkillFileStat): bool ); } -function listWatchableSkillDirs(rootPath: string): string[] { +function listWatchableSkillDirs(root: SkillSourceRoot): string[] { + const { rootPath } = root; try { return readdirSync(rootPath, { withFileTypes: true }) - .filter((child) => child.isDirectory()) + .filter((child) => { + if (child.isDirectory()) return true; + if (!child.isSymbolicLink() || !allowsDirectorySymlinkOutsideRoot(root)) return false; + try { + // Watch linked directories even before they contain a SKILL.md. + return statSync(path.join(rootPath, child.name)).isDirectory(); + } catch { + return false; + } + }) .map((child) => path.join(rootPath, child.name)) .sort(); } catch { diff --git a/packages/agent-modules/skills/src/types.ts b/packages/agent-modules/skills/src/types.ts index 150617e0..841c158e 100644 --- a/packages/agent-modules/skills/src/types.ts +++ b/packages/agent-modules/skills/src/types.ts @@ -7,6 +7,8 @@ export interface SkillSourceRoot { rootPath: string; priority?: number; external?: boolean; + /** Follow directory links outside the root for configured compatibility sources. */ + allowDirectorySymlinksOutsideRoot?: boolean; } export type SkillDiagnosticLevel = 'warning' | 'error'; diff --git a/packages/local-runtime/src/skills/roots.ts b/packages/local-runtime/src/skills/roots.ts index 27dec24d..6b3afd7c 100644 --- a/packages/local-runtime/src/skills/roots.ts +++ b/packages/local-runtime/src/skills/roots.ts @@ -134,6 +134,7 @@ function externalSkillRoot( rootPath, priority: sourceConfig.priority + priorityBonus, external: true, + allowDirectorySymlinksOutsideRoot: true, }; } diff --git a/packages/local-runtime/test/unit/skill-directory-symlinks.test.ts b/packages/local-runtime/test/unit/skill-directory-symlinks.test.ts new file mode 100644 index 00000000..68067f42 --- /dev/null +++ b/packages/local-runtime/test/unit/skill-directory-symlinks.test.ts @@ -0,0 +1,233 @@ +import { mkdtemp, mkdir, realpath, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { dirname, join, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { DEFAULT_SKILLS_CONFIG } from '@mavis/config'; +import { createSkillRegistry, type SkillRegistryWatcher } from '@mavis/skills'; +import { readConfiguredSkillRoots } from '../../src/skills/roots.js'; + +let fixture: string; +let workspace: string; +let watcher: SkillRegistryWatcher | undefined; + +beforeEach(async () => { + fixture = await realpath(await mkdtemp(join(tmpdir(), 'mcode-skill-links-'))); + workspace = join(fixture, 'workspace'); + await mkdir(join(workspace, '.git'), { recursive: true }); +}); + +afterEach(async () => { + watcher?.close(); + watcher = undefined; + await rm(fixture, { recursive: true, force: true }); +}); + +function workspaceRoots() { + return readConfiguredSkillRoots( + { dataDir: join(fixture, 'data'), provider: {} }, + 'mavis', + workspace, + ).filter((root) => root.kind === 'workspace'); +} + +async function writeSkill(dir: string, name = 'linked', body = 'Initial instructions') { + await mkdir(dir, { recursive: true }); + await writeFile( + join(dir, 'SKILL.md'), + `---\nname: ${name}\ndescription: Linked skill\n---\n# Linked skill\n${body}\n`, + ); +} + +async function linkDirectory(target: string, link: string, relativeTarget = false) { + await mkdir(dirname(link), { recursive: true }); + await symlink( + relativeTarget && process.platform !== 'win32' ? relative(dirname(link), target) : target, + link, + process.platform === 'win32' ? 'junction' : 'dir', + ); +} + +describe('configured workspace skill directory links', () => { + it.each(['.agents', '.claude', '.minimax'])( + 'discovers a linked %s/skills root', + async (source) => { + const target = join(workspace, 'skills', 'linked'); + await writeSkill(target); + await linkDirectory(dirname(target), join(workspace, source, 'skills'), true); + + const registry = await createSkillRegistry(workspaceRoots()); + const [skill] = registry.getAvailableSkills(); + expect(skill?.name).toBe('linked'); + expect(skill?.skillDir).toBe(target); + expect(registry.readByLocationUri(skill!.locationUri)).toContain('Initial instructions'); + expect(registry.getSnapshot()?.diagnostics).not.toEqual( + expect.arrayContaining([expect.objectContaining({ code: 'skill_outside_root' })]), + ); + }, + ); + + it.each([false, true])( + 'discovers child directory links (relative: %s) outside the workspace', + async (relativeTarget) => { + const target = join(fixture, 'shared', 'linked'); + const link = join(workspace, '.agents', 'skills', 'linked'); + await writeSkill(target); + await linkDirectory(target, link, relativeTarget); + + const registry = await createSkillRegistry(workspaceRoots()); + const [skill] = registry.getAvailableSkills(); + expect(skill).toMatchObject({ + name: 'linked', + skillDir: target, + entryDir: link, + }); + expect(fileURLToPath(skill!.locationUri.replace(/^files:/, 'file:'))).toBe( + join(target, 'SKILL.md'), + ); + }, + ); + + it('deduplicates shared root aliases using existing source priority', async () => { + const target = join(workspace, 'skills', 'linked'); + await writeSkill(target); + await linkDirectory(dirname(target), join(workspace, '.agents', 'skills'), true); + await linkDirectory(dirname(target), join(workspace, '.claude', 'skills'), true); + + const registry = await createSkillRegistry(workspaceRoots()); + expect(registry.getAvailableSkills()).toHaveLength(1); + expect(registry.getAvailableSkills()[0]?.rootId).toContain('workspace-cc:'); + expect(registry.getSnapshot()?.entries).toHaveLength(2); + }); + + it('retains external-source disable controls', () => { + const config = { + dataDir: join(fixture, 'data'), + provider: {}, + skills: { + external: { ...DEFAULT_SKILLS_CONFIG.external, enabled: false }, + }, + }; + expect( + readConfiguredSkillRoots(config, 'mavis', workspace).filter((root) => root.external), + ).toEqual([]); + config.skills.external.enabled = true; + config.skills.external.sources = { + ...DEFAULT_SKILLS_CONFIG.external.sources, + 'workspace-agents': { enabled: false, priority: 55 }, + }; + expect( + readConfiguredSkillRoots(config, 'mavis', workspace).some((root) => + root.id.startsWith('external:workspace-agents:'), + ), + ).toBe(false); + }); + + it('refreshes a retargeted child link without retaining cached content', async () => { + const first = join(fixture, 'first'); + const second = join(fixture, 'second'); + const link = join(workspace, '.agents', 'skills', 'linked'); + await writeSkill(first, 'linked', 'First instructions'); + await writeSkill(second, 'linked', 'Second instructions'); + await linkDirectory(first, link); + const registry = await createSkillRegistry(workspaceRoots()); + expect(registry.getAvailableSkills()[0]?.content).toContain('First instructions'); + await rm(link); + await linkDirectory(second, link); + await registry.refresh(); + expect(registry.getAvailableSkills()[0]).toMatchObject({ + skillDir: second, + entryDir: link, + }); + expect(registry.getAvailableSkills()[0]?.content).toContain('Second instructions'); + }); + + it('watches linked directories before SKILL.md exists and reloads later edits', async () => { + const target = join(fixture, 'shared', 'linked'); + await mkdir(target, { recursive: true }); + await linkDirectory(target, join(workspace, '.agents', 'skills', 'linked')); + const registry = await createSkillRegistry(workspaceRoots()); + expect(registry.getAvailableSkills()).toEqual([]); + const onChange = vi.fn(); + watcher = registry.watch({ onChange }); + + // Native watchers may finish arming after watch() returns. Observe a real + // event in the linked target before making the one-shot SKILL.md write. + await vi.waitFor( + async () => { + if (onChange.mock.calls.length === 0) { + await writeFile(join(target, '.watch-ready'), String(Date.now())); + } + expect(onChange).toHaveBeenCalled(); + }, + { timeout: 4000, interval: 250 }, + ); + expect(registry.getAvailableSkills()).toEqual([]); + onChange.mockClear(); + await writeSkill(target); + await vi.waitFor( + () => { + expect(onChange).toHaveBeenCalled(); + expect(registry.getAvailableSkills()[0]?.content).toContain('Initial instructions'); + }, + { timeout: 4000 }, + ); + onChange.mockClear(); + await writeSkill(target, 'linked', 'Updated instructions'); + await vi.waitFor( + () => { + expect(onChange).toHaveBeenCalled(); + expect(registry.getAvailableSkills()[0]?.content).toContain('Updated instructions'); + }, + { timeout: 4000 }, + ); + }, 15000); + + it.skipIf(process.platform === 'win32')( + 'skips broken, cyclic and non-directory links without losing valid skills', + async () => { + const root = join(workspace, '.agents', 'skills'); + await writeSkill(join(root, 'valid'), 'valid'); + await symlink(join(fixture, 'missing'), join(root, 'broken')); + await symlink('cycle', join(root, 'cycle')); + await writeFile(join(fixture, 'plain.txt'), 'Not a directory'); + await symlink(join(fixture, 'plain.txt'), join(root, 'file')); + const registry = await createSkillRegistry(workspaceRoots()); + expect(registry.getAvailableSkills().map((skill) => skill.name)).toEqual(['valid']); + expect(registry.getSnapshot()?.diagnostics).toEqual( + expect.arrayContaining([ + expect.objectContaining({ code: 'skill_directory_link_unreadable' }), + expect.objectContaining({ + code: 'skill_directory_link_not_directory', + }), + ]), + ); + }, + ); + + it.skipIf(process.platform === 'win32')('continues rejecting SKILL.md file links', async () => { + const target = join(fixture, 'shared'); + await writeSkill(target); + const skillDir = join(workspace, '.agents', 'skills', 'linked'); + await mkdir(skillDir, { recursive: true }); + await symlink(join(target, 'SKILL.md'), join(skillDir, 'SKILL.md')); + const registry = await createSkillRegistry(workspaceRoots()); + expect(registry.getAvailableSkills()).toEqual([]); + expect(registry.getSnapshot()?.diagnostics).toContainEqual( + expect.objectContaining({ code: 'skill_symlink_rejected' }), + ); + }); + + it.each(['project', 'workspace'] as const)('keeps unopted %s roots bounded', async (kind) => { + const target = join(fixture, 'shared', 'linked'); + const root = join(workspace, 'bounded'); + await writeSkill(target); + await linkDirectory(target, join(root, 'linked')); + const registry = await createSkillRegistry([{ id: 'bounded', kind, rootPath: root }]); + expect(registry.getAvailableSkills()).toEqual([]); + expect(registry.getSnapshot()?.diagnostics).toContainEqual( + expect.objectContaining({ code: 'skill_outside_root' }), + ); + }); +}); diff --git a/release/public-source.json b/release/public-source.json index d795f506..a07638a9 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -2656,6 +2656,7 @@ "packages/local-runtime/test/unit/local-thread-goal-wiring.test.ts", "packages/local-runtime/test/unit/metrics-telemetry-privacy.test.ts", "packages/local-runtime/test/unit/session-diff-fallback.test.ts", + "packages/local-runtime/test/unit/skill-directory-symlinks.test.ts", "packages/local-runtime/test/unit/thread-goal-kickoff-host.test.ts", "packages/local-runtime/test/unit/thread-goal-verifier-contract.test.ts", "packages/local-runtime/test/unit/thread-goal/contract.test.ts", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index 40ae8d55..9ddb86ca 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -5,6 +5,7 @@ "test/windows-contract.test.mjs" ], "capability": [ + "packages/local-runtime/test/unit/skill-directory-symlinks.test.ts", "packages/agent-core/test/unit/bash-subprocess-env.test.ts", "packages/agent-modules/permission/test/unit/permission/bash-policy-regressions.test.ts", "packages/agent-tools/src/shared/replace-all-edit.test.ts",