From cc0e3f74a55a91377623eb6c7786b19c6d5e89da Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Tue, 12 May 2026 14:18:44 +0200 Subject: [PATCH] refactor(client-mapping): single agent-host registry as source of truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CLIENT_MAPPINGS` and `USER_CLIENT_MAPPINGS` were two near-duplicate ~130-line records that had already drifted (windsurf's user-scope path differs from its project-scope path, but the divergence was hard to spot in review). Consolidates both into a single `AGENT_HOSTS` array — one entry per host, with paired `project` and `user` mappings. - `AgentHost` interface and `AGENT_HOSTS` const list every supported client exactly once. Pattern modelled after `cli/cli`'s `internal/skills/registry/registry.go::AgentHost`. - `CLIENT_MAPPINGS` / `USER_CLIENT_MAPPINGS` are now derived via `Object.fromEntries(AGENT_HOSTS.map(...))` so future drift between project and user maps is impossible by construction. - New helpers: `findHostById`, `getMapping(id, scope)`, `uniqueProjectSkillsPaths`, `agentHelpList`. - Compile-time `_HostsCoverEveryClient` assertion + a runtime test that walks every `ClientTypeSchema` value catches the "added to enum but forgot to register" bug class. - Intentional deltas (`windsurf` and `copilot` differing between scopes) preserved and tested explicitly so future reviewers see them as deliberate. All existing call sites (`CLIENT_MAPPINGS.claude`, etc.) keep working — the exports are the same shape, just frozen and derived. Optional follow-up: migrate consumers to `getMapping(id, scope)` and drop the derived records. Closes #377 --- src/models/client-mapping.ts | 528 +++++++++++++---------- tests/unit/models/client-mapping.test.ts | 48 +++ 2 files changed, 349 insertions(+), 227 deletions(-) diff --git a/src/models/client-mapping.ts b/src/models/client-mapping.ts index 018d9838..d56b447b 100644 --- a/src/models/client-mapping.ts +++ b/src/models/client-mapping.ts @@ -1,4 +1,4 @@ -import type { ClientType } from './workspace-config.js'; +import { ClientTypeSchema, type ClientType } from './workspace-config.js'; /** * Client-specific path and file configuration @@ -16,124 +16,300 @@ export interface ClientMapping { } /** - * Client path mappings for all supported AI clients + * Single source of truth for every supported AI client/agent host. + * + * Each entry pairs a project-scope mapping (paths relative to the project root) + * with a user-scope mapping (paths relative to ~). Most hosts use identical + * paths in both scopes; a few — notably `copilot`, `windsurf`, and `vscode` — + * intentionally diverge. Keeping both maps on one entry surfaces those + * differences at a glance instead of forcing reviewers to diff two ~130-line + * records. + * + * The legacy `CLIENT_MAPPINGS` and `USER_CLIENT_MAPPINGS` records below are + * derived from this array so existing call sites keep working while + * `getMapping(id, scope)` becomes the preferred accessor going forward. + * + * Pattern is modelled after `cli/cli`'s `internal/skills/registry/registry.go`. + */ +export interface AgentHost { + id: ClientType; + /** Display name used in help / sync output (e.g. "Windsurf"). */ + name: string; + project: ClientMapping; + user: ClientMapping; +} + +export const AGENT_HOSTS: readonly AgentHost[] = [ + { + id: 'claude', + name: 'Claude Code', + project: { + commandsPath: '.claude/commands/', + skillsPath: '.claude/skills/', + agentsPath: '.claude/agents/', + agentFile: 'CLAUDE.md', + agentFileFallback: 'AGENTS.md', + hooksPath: '.claude/hooks/', + }, + user: { + commandsPath: '.claude/commands/', + skillsPath: '.claude/skills/', + agentsPath: '.claude/agents/', + agentFile: 'CLAUDE.md', + agentFileFallback: 'AGENTS.md', + hooksPath: '.claude/hooks/', + }, + }, + { + id: 'copilot', + name: 'GitHub Copilot', + project: { + skillsPath: '.github/skills/', + agentsPath: '.github/agents/', + hooksPath: '.github/hooks/', + agentFile: 'AGENTS.md', + githubPath: '.github/', + }, + user: { + // User-scope Copilot stores under `.copilot/` because `.github/` is + // owned by individual repositories. + skillsPath: '.copilot/skills/', + agentsPath: '.copilot/agents/', + hooksPath: '.copilot/hooks/', + agentFile: 'AGENTS.md', + githubPath: '.copilot/', + }, + }, + { + id: 'codex', + name: 'Codex', + project: { skillsPath: '.codex/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.codex/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'cursor', + name: 'Cursor', + project: { skillsPath: '.cursor/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.cursor/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'opencode', + name: 'OpenCode', + project: { + commandsPath: '.opencode/commands/', + skillsPath: '.opencode/skills/', + agentFile: 'AGENTS.md', + }, + user: { + commandsPath: '.opencode/commands/', + skillsPath: '.opencode/skills/', + agentFile: 'AGENTS.md', + }, + }, + { + id: 'gemini', + name: 'Gemini', + project: { + skillsPath: '.gemini/skills/', + agentFile: 'GEMINI.md', + agentFileFallback: 'AGENTS.md', + }, + user: { + skillsPath: '.gemini/skills/', + agentFile: 'GEMINI.md', + agentFileFallback: 'AGENTS.md', + }, + }, + { + id: 'factory', + name: 'Factory', + project: { + skillsPath: '.factory/skills/', + agentFile: 'AGENTS.md', + hooksPath: '.factory/hooks/', + }, + user: { + skillsPath: '.factory/skills/', + agentFile: 'AGENTS.md', + hooksPath: '.factory/hooks/', + }, + }, + { + id: 'ampcode', + name: 'AmpCode', + project: { skillsPath: '.ampcode/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.ampcode/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'vscode', + name: 'VS Code', + // Defaults to the canonical universal location at both scopes. The + // copilot-sibling override is applied dynamically by resolveClientMappings. + project: { skillsPath: '.agents/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.agents/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'openclaw', + name: 'OpenClaw', + project: { skillsPath: 'skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: 'skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'windsurf', + name: 'Windsurf', + project: { skillsPath: '.windsurf/skills/', agentFile: 'AGENTS.md' }, + // Windsurf's user-scope home is the Codeium parent dir, not `.windsurf/`. + // Surfaced explicitly so the divergence is obvious in code review. + user: { skillsPath: '.codeium/windsurf/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'cline', + name: 'Cline', + project: { skillsPath: '.cline/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.cline/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'continue', + name: 'Continue', + project: { skillsPath: '.continue/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.continue/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'roo', + name: 'Roo Code', + project: { skillsPath: '.roo/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.roo/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'kilo', + name: 'Kilo Code', + project: { skillsPath: '.kilocode/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.kilocode/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'trae', + name: 'Trae', + project: { skillsPath: '.trae/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.trae/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'augment', + name: 'Augment', + project: { skillsPath: '.augment/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.augment/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'zencoder', + name: 'Zencoder', + project: { skillsPath: '.zencoder/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.zencoder/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'junie', + name: 'Junie', + project: { skillsPath: '.junie/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.junie/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'openhands', + name: 'OpenHands', + project: { skillsPath: '.openhands/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.openhands/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'kiro', + name: 'Kiro', + project: { skillsPath: '.kiro/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.kiro/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'replit', + name: 'Replit', + project: { skillsPath: '.replit/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.replit/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'kimi', + name: 'Kimi', + project: { skillsPath: '.kimi/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.kimi/skills/', agentFile: 'AGENTS.md' }, + }, + { + id: 'universal', + name: 'Universal', + project: { skillsPath: '.agents/skills/', agentFile: 'AGENTS.md' }, + user: { skillsPath: '.agents/skills/', agentFile: 'AGENTS.md' }, + }, +] as const; + +/** + * Look up an agent host by ClientType id. + * + * Returns the canonical entry; callers wanting the legacy `CLIENT_MAPPINGS` + * shape should use `getMapping(id, scope)` instead. + */ +export function findHostById(id: ClientType): AgentHost | undefined { + return AGENT_HOSTS.find((h) => h.id === id); +} + +/** + * Resolve the `ClientMapping` for a given (client, scope) pair. + * + * Falls back to throwing rather than returning undefined: every ClientType + * value is guaranteed to have a host entry (enforced by + * `client-mapping.test.ts`). Returning undefined would silently mask the + * "added to enum but not registered" bug we just removed. */ +export function getMapping( + id: ClientType, + scope: 'project' | 'user', +): ClientMapping { + const host = findHostById(id); + if (!host) { + throw new Error(`Unknown agent host: ${id} (no entry in AGENT_HOSTS)`); + } + return scope === 'user' ? host.user : host.project; +} + +/** + * The set of distinct skills paths used at project scope. Useful for the + * dedup logic that the symlink-mode sync runs against `.agents/skills/`. + */ +export function uniqueProjectSkillsPaths(): string[] { + return Array.from(new Set(AGENT_HOSTS.map((h) => h.project.skillsPath))); +} + +/** + * Render an agent help list: `" — "` per host, alphabetised. + * Used by user-facing help output. + */ +export function agentHelpList(): string { + return [...AGENT_HOSTS] + .sort((a, b) => a.id.localeCompare(b.id)) + .map((h) => `${h.id} — ${h.name}`) + .join('\n'); +} + /** * Project-level client path mappings for all supported AI clients. * Paths are relative to the project root directory. * - * The 'universal' and 'vscode' clients default to .agents/skills/. - * When copilot is also configured, vscode follows copilot's paths - * via resolveClientMappings(). + * Derived from `AGENT_HOSTS` so it can never drift from the user-scope record. + * Kept as a separate export for backward compatibility with existing call sites. */ -export const CLIENT_MAPPINGS: Record = { - claude: { - commandsPath: '.claude/commands/', - skillsPath: '.claude/skills/', - agentsPath: '.claude/agents/', - agentFile: 'CLAUDE.md', - agentFileFallback: 'AGENTS.md', - hooksPath: '.claude/hooks/', - }, - copilot: { - skillsPath: '.github/skills/', - agentsPath: '.github/agents/', - hooksPath: '.github/hooks/', - agentFile: 'AGENTS.md', - githubPath: '.github/', - }, - codex: { - skillsPath: '.codex/skills/', - agentFile: 'AGENTS.md', - }, - cursor: { - skillsPath: '.cursor/skills/', - agentFile: 'AGENTS.md', - }, - opencode: { - commandsPath: '.opencode/commands/', - skillsPath: '.opencode/skills/', - agentFile: 'AGENTS.md', - }, - gemini: { - skillsPath: '.gemini/skills/', - agentFile: 'GEMINI.md', - agentFileFallback: 'AGENTS.md', - }, - factory: { - skillsPath: '.factory/skills/', - agentFile: 'AGENTS.md', - hooksPath: '.factory/hooks/', - }, - ampcode: { - skillsPath: '.ampcode/skills/', - agentFile: 'AGENTS.md', - }, - vscode: { - skillsPath: '.agents/skills/', - agentFile: 'AGENTS.md', - }, - openclaw: { - skillsPath: 'skills/', - agentFile: 'AGENTS.md', - }, - windsurf: { - skillsPath: '.windsurf/skills/', - agentFile: 'AGENTS.md', - }, - cline: { - skillsPath: '.cline/skills/', - agentFile: 'AGENTS.md', - }, - continue: { - skillsPath: '.continue/skills/', - agentFile: 'AGENTS.md', - }, - roo: { - skillsPath: '.roo/skills/', - agentFile: 'AGENTS.md', - }, - kilo: { - skillsPath: '.kilocode/skills/', - agentFile: 'AGENTS.md', - }, - trae: { - skillsPath: '.trae/skills/', - agentFile: 'AGENTS.md', - }, - augment: { - skillsPath: '.augment/skills/', - agentFile: 'AGENTS.md', - }, - zencoder: { - skillsPath: '.zencoder/skills/', - agentFile: 'AGENTS.md', - }, - junie: { - skillsPath: '.junie/skills/', - agentFile: 'AGENTS.md', - }, - openhands: { - skillsPath: '.openhands/skills/', - agentFile: 'AGENTS.md', - }, - kiro: { - skillsPath: '.kiro/skills/', - agentFile: 'AGENTS.md', - }, - replit: { - skillsPath: '.replit/skills/', - agentFile: 'AGENTS.md', - }, - kimi: { - skillsPath: '.kimi/skills/', - agentFile: 'AGENTS.md', - }, - universal: { - skillsPath: '.agents/skills/', - agentFile: 'AGENTS.md', - }, -}; +export const CLIENT_MAPPINGS: Record = Object.freeze( + Object.fromEntries(AGENT_HOSTS.map((h) => [h.id, h.project])), +) as Record; + +/** + * User-level client path mappings for all supported AI clients. + * Paths are relative to the user's home directory (~/). + * + * Derived from `AGENT_HOSTS`. See note on `CLIENT_MAPPINGS`. + */ +export const USER_CLIENT_MAPPINGS: Record = Object.freeze( + Object.fromEntries(AGENT_HOSTS.map((h) => [h.id, h.user])), +) as Record; /** * The canonical skills path used by the universal client. @@ -150,120 +326,6 @@ export function isUniversalClient(client: ClientType): boolean { return client === 'universal'; } -/** - * User-level client path mappings for all supported AI clients. - * Paths are relative to the user's home directory (~/). - * Used when plugins are installed with --scope user. - */ -export const USER_CLIENT_MAPPINGS: Record = { - claude: { - commandsPath: '.claude/commands/', - skillsPath: '.claude/skills/', - agentsPath: '.claude/agents/', - agentFile: 'CLAUDE.md', - agentFileFallback: 'AGENTS.md', - hooksPath: '.claude/hooks/', - }, - copilot: { - skillsPath: '.copilot/skills/', - agentsPath: '.copilot/agents/', - hooksPath: '.copilot/hooks/', - agentFile: 'AGENTS.md', - githubPath: '.copilot/', - }, - codex: { - skillsPath: '.codex/skills/', - agentFile: 'AGENTS.md', - }, - cursor: { - skillsPath: '.cursor/skills/', - agentFile: 'AGENTS.md', - }, - opencode: { - commandsPath: '.opencode/commands/', - skillsPath: '.opencode/skills/', - agentFile: 'AGENTS.md', - }, - gemini: { - skillsPath: '.gemini/skills/', - agentFile: 'GEMINI.md', - agentFileFallback: 'AGENTS.md', - }, - factory: { - skillsPath: '.factory/skills/', - agentFile: 'AGENTS.md', - hooksPath: '.factory/hooks/', - }, - ampcode: { - skillsPath: '.ampcode/skills/', - agentFile: 'AGENTS.md', - }, - vscode: { - skillsPath: '.agents/skills/', - agentFile: 'AGENTS.md', - }, - openclaw: { - skillsPath: 'skills/', - agentFile: 'AGENTS.md', - }, - windsurf: { - skillsPath: '.codeium/windsurf/skills/', - agentFile: 'AGENTS.md', - }, - cline: { - skillsPath: '.cline/skills/', - agentFile: 'AGENTS.md', - }, - continue: { - skillsPath: '.continue/skills/', - agentFile: 'AGENTS.md', - }, - roo: { - skillsPath: '.roo/skills/', - agentFile: 'AGENTS.md', - }, - kilo: { - skillsPath: '.kilocode/skills/', - agentFile: 'AGENTS.md', - }, - trae: { - skillsPath: '.trae/skills/', - agentFile: 'AGENTS.md', - }, - augment: { - skillsPath: '.augment/skills/', - agentFile: 'AGENTS.md', - }, - zencoder: { - skillsPath: '.zencoder/skills/', - agentFile: 'AGENTS.md', - }, - junie: { - skillsPath: '.junie/skills/', - agentFile: 'AGENTS.md', - }, - openhands: { - skillsPath: '.openhands/skills/', - agentFile: 'AGENTS.md', - }, - kiro: { - skillsPath: '.kiro/skills/', - agentFile: 'AGENTS.md', - }, - replit: { - skillsPath: '.replit/skills/', - agentFile: 'AGENTS.md', - }, - kimi: { - skillsPath: '.kimi/skills/', - agentFile: 'AGENTS.md', - }, - universal: { - skillsPath: '.agents/skills/', - agentFile: 'AGENTS.md', - }, -}; - /** * Resolve vscode client mapping based on sibling clients. * When copilot is present, vscode follows copilot's paths. @@ -302,3 +364,15 @@ export const CLIENT_DISPLAY_ALIASES: Partial> = { export function getDisplayName(client: string): string { return CLIENT_DISPLAY_ALIASES[client as ClientType] ?? client; } + +// Compile-time sanity: every ClientType value must have exactly one host entry. +// Runtime coverage is verified by `client-mapping.test.ts`. +type _HostsCoverEveryClient = Exclude< + ClientType, + (typeof AGENT_HOSTS)[number]['id'] +> extends never + ? true + : never; +const _check: _HostsCoverEveryClient = true; +void _check; +void ClientTypeSchema; diff --git a/tests/unit/models/client-mapping.test.ts b/tests/unit/models/client-mapping.test.ts index 063740bf..0e41ed47 100644 --- a/tests/unit/models/client-mapping.test.ts +++ b/tests/unit/models/client-mapping.test.ts @@ -1,10 +1,15 @@ import { describe, expect, it, test } from 'bun:test'; import { + AGENT_HOSTS, CLIENT_MAPPINGS, USER_CLIENT_MAPPINGS, + findHostById, + getMapping, resolveClientMappings, getDisplayName, + uniqueProjectSkillsPaths, } from '../../../src/models/client-mapping.js'; +import { ClientTypeSchema } from '../../../src/models/workspace-config.js'; describe('CLIENT_MAPPINGS', () => { test('defines project-level paths for all supported clients', () => { @@ -246,3 +251,46 @@ describe('getDisplayName', () => { expect(getDisplayName('codex')).toBe('codex'); }); }); + +describe('AGENT_HOSTS (single source of truth)', () => { + it('covers every ClientType exactly once', () => { + const ids = new Set(AGENT_HOSTS.map((h) => h.id)); + expect(ids.size).toBe(AGENT_HOSTS.length); // no duplicates + const enumValues = new Set(ClientTypeSchema.options); + expect(ids.size).toBe(enumValues.size); + for (const value of enumValues) { + expect(ids.has(value)).toBe(true); + } + }); + + it('findHostById returns the entry for known clients and undefined otherwise', () => { + expect(findHostById('claude')?.id).toBe('claude'); + // Casting to bypass the strict ClientType for the negative case. + expect(findHostById('nonexistent' as 'claude')).toBeUndefined(); + }); + + it('getMapping yields identical content to the derived legacy records for every (client, scope) pair', () => { + for (const id of ClientTypeSchema.options) { + expect(getMapping(id, 'project')).toEqual(CLIENT_MAPPINGS[id]); + expect(getMapping(id, 'user')).toEqual(USER_CLIENT_MAPPINGS[id]); + } + }); + + it('preserves intentional windsurf project/user delta', () => { + expect(getMapping('windsurf', 'project').skillsPath).toBe('.windsurf/skills/'); + expect(getMapping('windsurf', 'user').skillsPath).toBe('.codeium/windsurf/skills/'); + }); + + it('preserves intentional copilot project/user delta', () => { + expect(getMapping('copilot', 'project').githubPath).toBe('.github/'); + expect(getMapping('copilot', 'user').githubPath).toBe('.copilot/'); + }); + + it('uniqueProjectSkillsPaths deduplicates shared paths (universal == vscode)', () => { + const paths = uniqueProjectSkillsPaths(); + // universal and vscode both default to .agents/skills/, so the set is + // smaller than the host count by at least one. + expect(paths.length).toBeLessThan(AGENT_HOSTS.length); + expect(paths).toContain('.agents/skills/'); + }); +});