diff --git a/src/core/sync.ts b/src/core/sync.ts index 02e20b9..4587b5e 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -33,7 +33,7 @@ import { type CopyResult, } from './transform.js'; import { updateAgentFiles } from './workspace-repo.js'; -import { CLIENT_MAPPINGS, USER_CLIENT_MAPPINGS, CANONICAL_SKILLS_PATH, isUniversalClient } from '../models/client-mapping.js'; +import { CLIENT_MAPPINGS, USER_CLIENT_MAPPINGS, CANONICAL_SKILLS_PATH, isUniversalClient, resolveClientMappings } from '../models/client-mapping.js'; import type { ClientMapping } from '../models/client-mapping.js'; import { resolveSkillNames, @@ -993,7 +993,7 @@ async function copyValidatedPlugin( syncMode: SyncMode = 'symlink', ): Promise { const copyResults: CopyResult[] = []; - const mappings = clientMappings ?? CLIENT_MAPPINGS; + const mappings = resolveClientMappings(clients, clientMappings ?? CLIENT_MAPPINGS); const clientList = clients; const exclude = validatedPlugin.exclude; @@ -1018,7 +1018,7 @@ async function copyValidatedPlugin( { dryRun, ...(skillNameMap && { skillNameMap }), - ...(clientMappings && { clientMappings }), + clientMappings: mappings, syncMode: 'copy', ...(exclude && { exclude }), }, @@ -1033,7 +1033,7 @@ async function copyValidatedPlugin( { dryRun, ...(skillNameMap && { skillNameMap }), - ...(clientMappings && { clientMappings }), + clientMappings: mappings, syncMode: 'symlink', canonicalSkillsPath: CANONICAL_SKILLS_PATH, ...(exclude && { exclude }), @@ -1054,7 +1054,7 @@ async function copyValidatedPlugin( { dryRun, ...(skillNameMap && { skillNameMap }), - ...(clientMappings && { clientMappings }), + clientMappings: mappings, syncMode: 'copy', ...(exclude && { exclude }), }, @@ -1640,7 +1640,8 @@ export async function syncWorkspace( ]; // Group by client and save state - const syncedFiles = collectSyncedPaths(allCopyResults, workspacePath, syncClients); + const resolvedMappings = resolveClientMappings(syncClients, CLIENT_MAPPINGS); + const syncedFiles = collectSyncedPaths(allCopyResults, workspacePath, syncClients, resolvedMappings); // When syncing a subset of clients, merge with existing state for non-targeted clients if (options.clients && previousState) { @@ -1773,7 +1774,8 @@ export async function syncUserWorkspace( const pluginResults = await Promise.all( validPlugins.map((vp) => { const skillNameMap = pluginSkillMaps.get(vp.resolved); - return copyValidatedPlugin(vp, homeDir, vp.clients, dryRun, skillNameMap, USER_CLIENT_MAPPINGS, syncMode); + const resolvedUserMappings = resolveClientMappings(vp.clients, USER_CLIENT_MAPPINGS); + return copyValidatedPlugin(vp, homeDir, vp.clients, dryRun, skillNameMap, resolvedUserMappings, syncMode); }), ); @@ -1908,7 +1910,8 @@ export async function syncUserWorkspace( // Save sync state (including MCP servers and native plugins) if (!dryRun) { const allCopyResults = pluginResults.flatMap((r) => r.copyResults); - const syncedFiles = collectSyncedPaths(allCopyResults, homeDir, syncClients, USER_CLIENT_MAPPINGS); + const resolvedUserMappings = resolveClientMappings(syncClients, USER_CLIENT_MAPPINGS); + const syncedFiles = collectSyncedPaths(allCopyResults, homeDir, syncClients, resolvedUserMappings); const nativePluginsState: Partial> = {}; const installedSet = new Set(nativeResult?.pluginsInstalled ?? []); diff --git a/src/models/client-mapping.ts b/src/models/client-mapping.ts index 622be42..6d37e75 100644 --- a/src/models/client-mapping.ts +++ b/src/models/client-mapping.ts @@ -22,8 +22,9 @@ export interface ClientMapping { * Project-level client path mappings for all supported AI clients. * Paths are relative to the project root directory. * - * Only the 'universal' client uses .agents/skills/. - * All other clients use provider-specific directories. + * The 'universal' and 'vscode' clients default to .agents/skills/. + * When copilot is also configured, vscode follows copilot's paths + * via resolveClientMappings(). */ export const CLIENT_MAPPINGS: Record = { claude: { @@ -67,9 +68,8 @@ export const CLIENT_MAPPINGS: Record = { agentFile: 'AGENTS.md', }, vscode: { - skillsPath: '.github/skills/', + skillsPath: '.agents/skills/', agentFile: 'AGENTS.md', - githubPath: '.github/', }, openclaw: { skillsPath: 'skills/', @@ -195,9 +195,8 @@ export const USER_CLIENT_MAPPINGS: Record = { agentFile: 'AGENTS.md', }, vscode: { - skillsPath: '.copilot/skills/', + skillsPath: '.agents/skills/', agentFile: 'AGENTS.md', - githubPath: '.copilot/', }, openclaw: { skillsPath: 'skills/', @@ -260,3 +259,24 @@ export const USER_CLIENT_MAPPINGS: Record = { agentFile: 'AGENTS.md', }, }; + +/** + * Resolve vscode client mapping based on sibling clients. + * When copilot is present, vscode follows copilot's paths. + * When copilot is absent, vscode defaults to .agents/ (universal behavior). + * + * Returns baseMappings unchanged if vscode is not in the clients list. + */ +export function resolveClientMappings( + clients: ClientType[], + baseMappings: Record, +): Record { + if (!clients.includes('vscode')) return baseMappings; + if (!clients.includes('copilot')) return baseMappings; + + // vscode follows copilot's mapping + return { + ...baseMappings, + vscode: { ...baseMappings.copilot }, + }; +} diff --git a/tests/unit/core/sync-dedup.test.ts b/tests/unit/core/sync-dedup.test.ts index b024a58..6ae2c92 100644 --- a/tests/unit/core/sync-dedup.test.ts +++ b/tests/unit/core/sync-dedup.test.ts @@ -5,14 +5,15 @@ import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { syncWorkspace, deduplicateClientsByPath, collectSyncedPaths } from '../../../src/core/sync.js'; import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; -import { CLIENT_MAPPINGS, USER_CLIENT_MAPPINGS } from '../../../src/models/client-mapping.js'; +import { CLIENT_MAPPINGS, USER_CLIENT_MAPPINGS, resolveClientMappings } from '../../../src/models/client-mapping.js'; import type { CopyResult } from '../../../src/core/transform.js'; describe('deduplicateClientsByPath', () => { - it('should group clients that share the same skillsPath', () => { - // copilot and vscode both use .github/skills/ + it('should group clients that share the same skillsPath after resolution', () => { + // After resolution, copilot and vscode both use .github/skills/ const clients = ['copilot', 'vscode'] as const; - const result = deduplicateClientsByPath([...clients], CLIENT_MAPPINGS); + const resolvedMappings = resolveClientMappings([...clients], CLIENT_MAPPINGS); + const result = deduplicateClientsByPath([...clients], resolvedMappings); // Should have only one representative client expect(result.representativeClients).toHaveLength(1); @@ -42,10 +43,11 @@ describe('deduplicateClientsByPath', () => { expect(result.clientGroups.get('codex')).toEqual(['codex']); }); - it('should handle mixed unique and shared paths', () => { - // claude (unique .claude/skills/), copilot+vscode (shared .github/skills/), codex (unique .codex/skills/) + it('should handle mixed unique and shared paths after resolution', () => { + // claude (unique .claude/skills/), copilot+vscode (shared .github/skills/ after resolution), codex (unique .codex/skills/) const clients = ['claude', 'copilot', 'vscode', 'codex'] as const; - const result = deduplicateClientsByPath([...clients], CLIENT_MAPPINGS); + const resolvedMappings = resolveClientMappings([...clients], CLIENT_MAPPINGS); + const result = deduplicateClientsByPath([...clients], resolvedMappings); // Should have 3 representative clients expect(result.representativeClients).toHaveLength(3); @@ -88,11 +90,12 @@ describe('deduplicateClientsByPath', () => { expect(result.clientGroups.get('claude')).toEqual(['claude']); }); - it('should group vscode with copilot (both use .github/skills/)', () => { + it('should group vscode with copilot after resolution', () => { const clients = ['copilot', 'vscode', 'codex'] as const; - const result = deduplicateClientsByPath([...clients], CLIENT_MAPPINGS); + const resolvedMappings = resolveClientMappings([...clients], CLIENT_MAPPINGS); + const result = deduplicateClientsByPath([...clients], resolvedMappings); - // copilot and vscode share .github/skills/, codex uses .codex/skills/ + // After resolution, copilot and vscode share .github/skills/, codex uses .codex/skills/ expect(result.representativeClients).toHaveLength(2); expect(result.representativeClients).toContain('copilot'); expect(result.representativeClients).toContain('codex'); @@ -106,11 +109,19 @@ describe('deduplicateClientsByPath', () => { expect(codexGroup).toHaveLength(1); expect(codexGroup).toContain('codex'); }); + + it('should not group vscode with copilot in unresolved CLIENT_MAPPINGS', () => { + const result = deduplicateClientsByPath(['copilot', 'vscode'], CLIENT_MAPPINGS); + // Without resolution, vscode uses .agents/skills/ and copilot uses .github/skills/ + expect(result.representativeClients).toHaveLength(2); + expect(result.representativeClients).toContain('copilot'); + expect(result.representativeClients).toContain('vscode'); + }); }); describe('collectSyncedPaths with shared paths', () => { - it('should track file for all clients sharing the same skillsPath', () => { - // copilot and vscode both use .github/skills/ + it('should track file for all clients sharing the same skillsPath after resolution', () => { + // After resolution, copilot and vscode both use .github/skills/ const copyResults: CopyResult[] = [ { source: '/some/plugin/skills/my-skill', @@ -120,7 +131,8 @@ describe('collectSyncedPaths with shared paths', () => { ]; const clients = ['copilot', 'vscode'] as const; - const result = collectSyncedPaths(copyResults, '/workspace', [...clients], CLIENT_MAPPINGS); + const resolvedMappings = resolveClientMappings([...clients], CLIENT_MAPPINGS); + const result = collectSyncedPaths(copyResults, '/workspace', [...clients], resolvedMappings); // Both clients should track the same skill expect(result.copilot).toContain('.github/skills/my-skill/'); @@ -337,3 +349,121 @@ syncMode: copy expect(existsSync(join(testDir, '.claude', 'skills', 'test-skill', 'SKILL.md'))).toBe(true); }); }); + +describe('syncWorkspace vscode artifact placement', () => { + let testDir: string; + + beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'allagents-vscode-test-')); + }); + + afterEach(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + async function createPluginWithSkill(name: string, skillName: string): Promise { + const pluginDir = join(testDir, name); + const skillDir = join(pluginDir, 'skills', skillName); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + `---\nname: ${skillName}\ndescription: A test skill\n---\n\n# ${skillName}`, + ); + return pluginDir; + } + + it('should place skills in .agents/ when vscode is the only client', async () => { + const pluginDir = await createPluginWithSkill('my-plugin', 'test-skill'); + + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ${pluginDir} +clients: + - vscode +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + expect(existsSync(join(testDir, '.agents', 'skills', 'test-skill', 'SKILL.md'))).toBe(true); + expect(existsSync(join(testDir, '.github', 'skills', 'test-skill', 'SKILL.md'))).toBe(false); + }); + + it('should place skills in .github/ when copilot and vscode are both configured', async () => { + const pluginDir = await createPluginWithSkill('my-plugin', 'test-skill'); + + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ${pluginDir} +clients: + - copilot + - vscode +syncMode: copy +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + expect(existsSync(join(testDir, '.github', 'skills', 'test-skill', 'SKILL.md'))).toBe(true); + // Should only copy once (deduped) + expect(result.totalCopied).toBe(1); + }); + + it('should place skills in .agents/ with .github symlink when universal + copilot + vscode', async () => { + const pluginDir = await createPluginWithSkill('my-plugin', 'test-skill'); + + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ${pluginDir} +clients: + - universal + - copilot + - vscode +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + // Canonical in .agents + expect(existsSync(join(testDir, '.agents', 'skills', 'test-skill', 'SKILL.md'))).toBe(true); + // .github should exist (symlink or copy from copilot+vscode) + expect(existsSync(join(testDir, '.github', 'skills', 'test-skill', 'SKILL.md'))).toBe(true); + }); + + it('should place skills in .agents/ when universal + vscode (no copilot)', async () => { + const pluginDir = await createPluginWithSkill('my-plugin', 'test-skill'); + + await mkdir(join(testDir, CONFIG_DIR), { recursive: true }); + await writeFile( + join(testDir, CONFIG_DIR, WORKSPACE_CONFIG_FILE), + ` +repositories: [] +plugins: + - ${pluginDir} +clients: + - universal + - vscode +`, + ); + + const result = await syncWorkspace(testDir); + expect(result.success).toBe(true); + expect(existsSync(join(testDir, '.agents', 'skills', 'test-skill', 'SKILL.md'))).toBe(true); + // Should NOT create .github since no copilot + expect(existsSync(join(testDir, '.github', 'skills'))).toBe(false); + // Should only copy once (deduped — both map to .agents) + expect(result.totalCopied).toBe(1); + }); +}); diff --git a/tests/unit/models/client-mapping.test.ts b/tests/unit/models/client-mapping.test.ts index b5d74ce..172b05c 100644 --- a/tests/unit/models/client-mapping.test.ts +++ b/tests/unit/models/client-mapping.test.ts @@ -1,5 +1,9 @@ -import { describe, expect, test } from 'bun:test'; -import { CLIENT_MAPPINGS, USER_CLIENT_MAPPINGS } from '../../../src/models/client-mapping.js'; +import { describe, expect, it, test } from 'bun:test'; +import { + CLIENT_MAPPINGS, + USER_CLIENT_MAPPINGS, + resolveClientMappings, +} from '../../../src/models/client-mapping.js'; describe('CLIENT_MAPPINGS', () => { test('defines project-level paths for all supported clients', () => { @@ -49,8 +53,9 @@ describe('CLIENT_MAPPINGS', () => { expect(CLIENT_MAPPINGS.ampcode.skillsPath).toBe('.ampcode/skills/'); }); - test('vscode uses provider-specific .github/skills/ path', () => { - expect(CLIENT_MAPPINGS.vscode.skillsPath).toBe('.github/skills/'); + test('vscode defaults to .agents/skills/ path', () => { + expect(CLIENT_MAPPINGS.vscode.skillsPath).toBe('.agents/skills/'); + expect(CLIENT_MAPPINGS.vscode.githubPath).toBeUndefined(); }); test('openclaw uses root-level skills/ path (no dot prefix)', () => { @@ -140,8 +145,9 @@ describe('USER_CLIENT_MAPPINGS', () => { expect(USER_CLIENT_MAPPINGS.ampcode.skillsPath).toBe('.ampcode/skills/'); }); - test('vscode uses provider-specific ~/.copilot/skills/ path', () => { - expect(USER_CLIENT_MAPPINGS.vscode.skillsPath).toBe('.copilot/skills/'); + test('vscode defaults to .agents/skills/ path', () => { + expect(USER_CLIENT_MAPPINGS.vscode.skillsPath).toBe('.agents/skills/'); + expect(USER_CLIENT_MAPPINGS.vscode.githubPath).toBeUndefined(); }); test('openclaw uses root-level skills/ path', () => { @@ -175,3 +181,55 @@ describe('USER_CLIENT_MAPPINGS', () => { } }); }); + +describe('resolveClientMappings', () => { + describe('project-level (CLIENT_MAPPINGS)', () => { + it('should default vscode to .agents/skills/ when no copilot', () => { + const resolved = resolveClientMappings(['vscode'], CLIENT_MAPPINGS); + expect(resolved.vscode.skillsPath).toBe('.agents/skills/'); + expect(resolved.vscode.githubPath).toBeUndefined(); + }); + + it('should resolve vscode to .github/skills/ when copilot is present', () => { + const resolved = resolveClientMappings(['copilot', 'vscode'], CLIENT_MAPPINGS); + expect(resolved.vscode.skillsPath).toBe('.github/skills/'); + expect(resolved.vscode.githubPath).toBe('.github/'); + }); + + it('should resolve vscode to .github/skills/ when both copilot and universal are present', () => { + const resolved = resolveClientMappings(['universal', 'copilot', 'vscode'], CLIENT_MAPPINGS); + expect(resolved.vscode.skillsPath).toBe('.github/skills/'); + expect(resolved.vscode.githubPath).toBe('.github/'); + }); + + it('should resolve vscode to .agents/skills/ when universal is present but not copilot', () => { + const resolved = resolveClientMappings(['universal', 'vscode'], CLIENT_MAPPINGS); + expect(resolved.vscode.skillsPath).toBe('.agents/skills/'); + expect(resolved.vscode.githubPath).toBeUndefined(); + }); + + it('should not modify non-vscode client mappings', () => { + const resolved = resolveClientMappings(['copilot', 'vscode', 'claude'], CLIENT_MAPPINGS); + expect(resolved.copilot).toEqual(CLIENT_MAPPINGS.copilot); + expect(resolved.claude).toEqual(CLIENT_MAPPINGS.claude); + }); + + it('should return baseMappings unchanged when vscode is not in clients', () => { + const resolved = resolveClientMappings(['copilot', 'claude'], CLIENT_MAPPINGS); + expect(resolved).toBe(CLIENT_MAPPINGS); // same reference + }); + }); + + describe('user-level (USER_CLIENT_MAPPINGS)', () => { + it('should default vscode to .agents/skills/ when no copilot', () => { + const resolved = resolveClientMappings(['vscode'], USER_CLIENT_MAPPINGS); + expect(resolved.vscode.skillsPath).toBe('.agents/skills/'); + }); + + it('should resolve vscode to .copilot/skills/ when copilot is present', () => { + const resolved = resolveClientMappings(['copilot', 'vscode'], USER_CLIENT_MAPPINGS); + expect(resolved.vscode.skillsPath).toBe('.copilot/skills/'); + expect(resolved.vscode.githubPath).toBe('.copilot/'); + }); + }); +});