Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/core/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -993,7 +993,7 @@ async function copyValidatedPlugin(
syncMode: SyncMode = 'symlink',
): Promise<PluginSyncResult> {
const copyResults: CopyResult[] = [];
const mappings = clientMappings ?? CLIENT_MAPPINGS;
const mappings = resolveClientMappings(clients, clientMappings ?? CLIENT_MAPPINGS);
const clientList = clients;

const exclude = validatedPlugin.exclude;
Expand All @@ -1018,7 +1018,7 @@ async function copyValidatedPlugin(
{
dryRun,
...(skillNameMap && { skillNameMap }),
...(clientMappings && { clientMappings }),
clientMappings: mappings,
syncMode: 'copy',
...(exclude && { exclude }),
},
Expand All @@ -1033,7 +1033,7 @@ async function copyValidatedPlugin(
{
dryRun,
...(skillNameMap && { skillNameMap }),
...(clientMappings && { clientMappings }),
clientMappings: mappings,
syncMode: 'symlink',
canonicalSkillsPath: CANONICAL_SKILLS_PATH,
...(exclude && { exclude }),
Expand All @@ -1054,7 +1054,7 @@ async function copyValidatedPlugin(
{
dryRun,
...(skillNameMap && { skillNameMap }),
...(clientMappings && { clientMappings }),
clientMappings: mappings,
syncMode: 'copy',
...(exclude && { exclude }),
},
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}),
);

Expand Down Expand Up @@ -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<Record<ClientType, string[]>> = {};
const installedSet = new Set(nativeResult?.pluginsInstalled ?? []);
Expand Down
32 changes: 26 additions & 6 deletions src/models/client-mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClientType, ClientMapping> = {
claude: {
Expand Down Expand Up @@ -67,9 +68,8 @@ export const CLIENT_MAPPINGS: Record<ClientType, ClientMapping> = {
agentFile: 'AGENTS.md',
},
vscode: {
skillsPath: '.github/skills/',
skillsPath: '.agents/skills/',
agentFile: 'AGENTS.md',
githubPath: '.github/',
},
openclaw: {
skillsPath: 'skills/',
Expand Down Expand Up @@ -195,9 +195,8 @@ export const USER_CLIENT_MAPPINGS: Record<ClientType, ClientMapping> = {
agentFile: 'AGENTS.md',
},
vscode: {
skillsPath: '.copilot/skills/',
skillsPath: '.agents/skills/',
agentFile: 'AGENTS.md',
githubPath: '.copilot/',
},
openclaw: {
skillsPath: 'skills/',
Expand Down Expand Up @@ -260,3 +259,24 @@ export const USER_CLIENT_MAPPINGS: Record<ClientType, ClientMapping> = {
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<ClientType, ClientMapping>,
): Record<ClientType, ClientMapping> {
if (!clients.includes('vscode')) return baseMappings;
if (!clients.includes('copilot')) return baseMappings;

// vscode follows copilot's mapping
return {
...baseMappings,
vscode: { ...baseMappings.copilot },
};
}
156 changes: 143 additions & 13 deletions tests/unit/core/sync-dedup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand All @@ -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',
Expand All @@ -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/');
Expand Down Expand Up @@ -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<string> {
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);
});
});
Loading